From b82afe37dc4c7b51bdb70df64bdfa121ac882a02 Mon Sep 17 00:00:00 2001 From: David Arenas Date: Mon, 28 Oct 2019 17:42:56 +0000 Subject: [PATCH] Bugfix/invalid cached highest column after column removed (#1195) * Call garbage collector after removing a column Otherwise callers of getHighestColumn get stale values * Update changelog --- CHANGELOG.md | 2 +- src/PhpSpreadsheet/Worksheet/Worksheet.php | 1 + .../Worksheet/WorksheetTest.php | 61 +++++++++++++++++++ 3 files changed, 63 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1aed31e4..56989e4b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,7 +13,7 @@ and this project adheres to [Semantic Versioning](https://semver.org). ### Fixed -- ... +- Call garbage collector after removing a column to prevent stale cached values ## [1.9.0] - 2019-08-17 diff --git a/src/PhpSpreadsheet/Worksheet/Worksheet.php b/src/PhpSpreadsheet/Worksheet/Worksheet.php index 7da5f693..2a7c529d 100644 --- a/src/PhpSpreadsheet/Worksheet/Worksheet.php +++ b/src/PhpSpreadsheet/Worksheet/Worksheet.php @@ -2154,6 +2154,7 @@ class Worksheet implements IComparable $this->getCellCollection()->removeColumn($highestColumn); $highestColumn = Coordinate::stringFromColumnIndex(Coordinate::columnIndexFromString($highestColumn) - 1); } + $this->garbageCollect(); } else { throw new Exception('Column references should not be numeric.'); } diff --git a/tests/PhpSpreadsheetTests/Worksheet/WorksheetTest.php b/tests/PhpSpreadsheetTests/Worksheet/WorksheetTest.php index eb2aa200..b6fcb57c 100644 --- a/tests/PhpSpreadsheetTests/Worksheet/WorksheetTest.php +++ b/tests/PhpSpreadsheetTests/Worksheet/WorksheetTest.php @@ -181,4 +181,65 @@ class WorksheetTest extends TestCase $worksheet->getCell('C1')->getValue() ); } + + public function removeColumnProvider(): array + { + return [ + 'Remove first column' => [ + [ + ['A1', 'B1', 'C1'], + ['A2', 'B2', 'C2'], + ], + 'A', + [ + ['B1', 'C1'], + ['B2', 'C2'], + ], + 'B', + ], + 'Remove middle column' => [ + [ + ['A1', 'B1', 'C1'], + ['A2', 'B2', 'C2'], + ], + 'B', + [ + ['A1', 'C1'], + ['A2', 'C2'], + ], + 'B', + ], + 'Remove last column' => [ + [ + ['A1', 'B1', 'C1'], + ['A2', 'B2', 'C2'], + ], + 'C', + [ + ['A1', 'B1'], + ['A2', 'B2'], + ], + 'B', + ], + ]; + } + + /** + * @dataProvider removeColumnProvider + */ + public function testRemoveColumn( + array $initialData, + string $columnToBeRemoved, + array $expectedData, + string $expectedHighestColumn + ) { + $spreadsheet = new Spreadsheet(); + $worksheet = $spreadsheet->getActiveSheet(); + $worksheet->fromArray($initialData); + + $worksheet->removeColumn($columnToBeRemoved); + + self::assertSame($expectedHighestColumn, $worksheet->getHighestColumn()); + self::assertSame($expectedData, $worksheet->toArray()); + } }