Skip to content

Commit 345e7f0

Browse files
oleibmanoleibman
authored
Change hash code for worksheet branch release222 (#4308)
* Change hash code for worksheet branch release210 Backport of PR #4207. * Retitling Clone Worksheets Backport of PR #4302.
1 parent cc41c3b commit 345e7f0

File tree

16 files changed

+248
-37
lines changed

16 files changed

+248
-37
lines changed

‎CHANGELOG.md

+12
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,18 @@ All notable changes to this project will be documented in this file.
55
The format is based on [Keep a Changelog](https://keepachangelog.com)
66
and this project adheres to [Semantic Versioning](https://semver.org).
77

8+
# TBD - 2.3.6
9+
10+
### Deprecated
11+
12+
- Worksheet::getHashCode is no longer needed.
13+
14+
### Fixed
15+
16+
- Change hash code for worksheet. Backport of [PR #4207](https://github.com/PHPOffice/PhpSpreadsheet/pull/4207)
17+
- Retitling cloned worksheets. Backport of [PR #4302](https://github.com/PHPOffice/PhpSpreadsheet/pull/4302)
18+
19+
820
# 2024-12-26 - 2.3.5
921

1022
### Deprecated

‎src/PhpSpreadsheet/Cell/Cell.php

+1-1
Original file line numberDiff line numberDiff line change
@@ -318,7 +318,7 @@ public function setValueExplicit(mixed $value, string $dataType = DataType::TYPE
318318
self::updateIfCellIsTableHeader($this->getParent()?->getParent(), $this, $oldValue, $value);
319319
$worksheet = $this->getWorksheet();
320320
$spreadsheet = $worksheet->getParent();
321-
if (isset($spreadsheet)) {
321+
if (isset($spreadsheet) && $spreadsheet->getIndex($worksheet, true) >= 0) {
322322
$originalSelected = $worksheet->getSelectedCells();
323323
$activeSheetIndex = $spreadsheet->getActiveSheetIndex();
324324
$style = $this->getStyle();

‎src/PhpSpreadsheet/ReferenceHelper.php

+2-2
Original file line numberDiff line numberDiff line change
@@ -921,7 +921,7 @@ private function updateNamedRange(DefinedName $definedName, Worksheet $worksheet
921921
{
922922
$cellAddress = $definedName->getValue();
923923
$asFormula = ($cellAddress[0] === '=');
924-
if ($definedName->getWorksheet() !== null && $definedName->getWorksheet()->getHashCode() === $worksheet->getHashCode()) {
924+
if ($definedName->getWorksheet() !== null && $definedName->getWorksheet()->getHashInt() === $worksheet->getHashInt()) {
925925
/**
926926
* If we delete the entire range that is referenced by a Named Range, MS Excel sets the value to #REF!
927927
* PhpSpreadsheet still only does a basic adjustment, so the Named Range will still reference Cells.
@@ -940,7 +940,7 @@ private function updateNamedRange(DefinedName $definedName, Worksheet $worksheet
940940

941941
private function updateNamedFormula(DefinedName $definedName, Worksheet $worksheet, string $beforeCellAddress, int $numberOfColumns, int $numberOfRows): void
942942
{
943-
if ($definedName->getWorksheet() !== null && $definedName->getWorksheet()->getHashCode() === $worksheet->getHashCode()) {
943+
if ($definedName->getWorksheet() !== null && $definedName->getWorksheet()->getHashInt() === $worksheet->getHashInt()) {
944944
/**
945945
* If we delete the entire range that is referenced by a Named Formula, MS Excel sets the value to #REF!
946946
* PhpSpreadsheet still only does a basic adjustment, so the Named Formula will still reference Cells.

‎src/PhpSpreadsheet/Spreadsheet.php

+20-4
Original file line numberDiff line numberDiff line change
@@ -511,7 +511,7 @@ public function getActiveSheet(): Worksheet
511511
public function createSheet(?int $sheetIndex = null): Worksheet
512512
{
513513
$newSheet = new Worksheet($this);
514-
$this->addSheet($newSheet, $sheetIndex);
514+
$this->addSheet($newSheet, $sheetIndex, true);
515515

516516
return $newSheet;
517517
}
@@ -532,8 +532,20 @@ public function sheetNameExists(string $worksheetName): bool
532532
* @param Worksheet $worksheet The worksheet to add
533533
* @param null|int $sheetIndex Index where sheet should go (0,1,..., or null for last)
534534
*/
535-
public function addSheet(Worksheet $worksheet, ?int $sheetIndex = null): Worksheet
535+
public function addSheet(Worksheet $worksheet, ?int $sheetIndex = null, bool $retitleIfNeeded = false): Worksheet
536536
{
537+
if ($retitleIfNeeded) {
538+
$title = $worksheet->getTitle();
539+
if ($this->sheetNameExists($title)) {
540+
$i = 1;
541+
$newTitle = "$title $i";
542+
while ($this->sheetNameExists($newTitle)) {
543+
++$i;
544+
$newTitle = "$title $i";
545+
}
546+
$worksheet->setTitle($newTitle);
547+
}
548+
}
537549
if ($this->sheetNameExists($worksheet->getTitle())) {
538550
throw new Exception(
539551
"Workbook already contains a worksheet named '{$worksheet->getTitle()}'. Rename this worksheet first."
@@ -657,13 +669,17 @@ public function getSheetByNameOrThrow(string $worksheetName): Worksheet
657669
*
658670
* @return int index
659671
*/
660-
public function getIndex(Worksheet $worksheet): int
672+
public function getIndex(Worksheet $worksheet, bool $noThrow = false): int
661673
{
674+
$wsHash = $worksheet->getHashInt();
662675
foreach ($this->workSheetCollection as $key => $value) {
663-
if ($value->getHashCode() === $worksheet->getHashCode()) {
676+
if ($value->getHashInt() === $wsHash) {
664677
return $key;
665678
}
666679
}
680+
if ($noThrow) {
681+
return -1;
682+
}
667683

668684
throw new Exception('Sheet does not exist.');
669685
}

‎src/PhpSpreadsheet/Worksheet/BaseDrawing.php

+1-1
Original file line numberDiff line numberDiff line change
@@ -416,7 +416,7 @@ public function getHashCode(): string
416416
return md5(
417417
$this->name
418418
. $this->description
419-
. (($this->worksheet === null) ? '' : $this->worksheet->getHashCode())
419+
. (($this->worksheet === null) ? '' : (string) $this->worksheet->getHashInt())
420420
. $this->coordinates
421421
. $this->offsetX
422422
. $this->offsetY

‎src/PhpSpreadsheet/Worksheet/Worksheet.php

+18-20
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@
2020
use PhpOffice\PhpSpreadsheet\Comment;
2121
use PhpOffice\PhpSpreadsheet\DefinedName;
2222
use PhpOffice\PhpSpreadsheet\Exception;
23-
use PhpOffice\PhpSpreadsheet\IComparable;
2423
use PhpOffice\PhpSpreadsheet\ReferenceHelper;
2524
use PhpOffice\PhpSpreadsheet\RichText\RichText;
2625
use PhpOffice\PhpSpreadsheet\Shared;
@@ -32,7 +31,7 @@
3231
use PhpOffice\PhpSpreadsheet\Style\Protection as StyleProtection;
3332
use PhpOffice\PhpSpreadsheet\Style\Style;
3433

35-
class Worksheet implements IComparable
34+
class Worksheet
3635
{
3736
// Break types
3837
public const BREAK_NONE = 0;
@@ -305,15 +304,10 @@ class Worksheet implements IComparable
305304
*/
306305
private ?Color $tabColor = null;
307306

308-
/**
309-
* Dirty flag.
310-
*/
311-
private bool $dirty = true;
312-
313307
/**
314308
* Hash.
315309
*/
316-
private string $hash;
310+
private int $hash;
317311

318312
/**
319313
* CodeName.
@@ -327,6 +321,7 @@ public function __construct(?Spreadsheet $parent = null, string $title = 'Worksh
327321
{
328322
// Set parent and title
329323
$this->parent = $parent;
324+
$this->hash = spl_object_id($this);
330325
$this->setTitle($title, false);
331326
// setTitle can change $pTitle
332327
$this->setCodeName($this->getTitle());
@@ -383,6 +378,12 @@ public function __destruct()
383378
unset($this->rowDimensions, $this->columnDimensions, $this->tableCollection, $this->drawingCollection, $this->chartCollection, $this->autoFilter);
384379
}
385380

381+
public function __wakeup(): void
382+
{
383+
$this->hash = spl_object_id($this);
384+
$this->parent = null;
385+
}
386+
386387
/**
387388
* Return the cell collection.
388389
*/
@@ -862,7 +863,7 @@ public function setTitle(string $title, bool $updateFormulaCellReferences = true
862863
// Syntax check
863864
self::checkSheetTitle($title);
864865

865-
if ($this->parent) {
866+
if ($this->parent && $this->parent->getIndex($this, true) >= 0) {
866867
// Is there already such sheet name?
867868
if ($this->parent->sheetNameExists($title)) {
868869
// Use name, but append with lowest possible integer
@@ -891,9 +892,8 @@ public function setTitle(string $title, bool $updateFormulaCellReferences = true
891892

892893
// Set title
893894
$this->title = $title;
894-
$this->dirty = true;
895895

896-
if ($this->parent && $this->parent->getCalculationEngine()) {
896+
if ($this->parent && $this->parent->getIndex($this, true) >= 0 && $this->parent->getCalculationEngine()) {
897897
// New title
898898
$newTitle = $this->getTitle();
899899
$this->parent->getCalculationEngine()
@@ -1026,7 +1026,6 @@ public function getProtection(): Protection
10261026
public function setProtection(Protection $protection): static
10271027
{
10281028
$this->protection = $protection;
1029-
$this->dirty = true;
10301029

10311030
return $this;
10321031
}
@@ -2994,7 +2993,7 @@ private function validateNamedRange(string $definedName, bool $returnNullIfInval
29942993

29952994
if ($namedRange->getLocalOnly()) {
29962995
$worksheet = $namedRange->getWorksheet();
2997-
if ($worksheet === null || $this->getHashCode() !== $worksheet->getHashCode()) {
2996+
if ($worksheet === null || $this->hash !== $worksheet->getHashInt()) {
29982997
if ($returnNullIfInvalid) {
29992998
return null;
30002999
}
@@ -3131,17 +3130,15 @@ public function garbageCollect(): static
31313130
}
31323131

31333132
/**
3134-
* Get hash code.
3135-
*
3136-
* @return string Hash code
3133+
* @deprecated 3.5.0 use getHashInt instead.
31373134
*/
31383135
public function getHashCode(): string
31393136
{
3140-
if ($this->dirty) {
3141-
$this->hash = md5($this->title . $this->autoFilter . ($this->protection->isProtectionEnabled() ? 't' : 'f') . __CLASS__);
3142-
$this->dirty = false;
3143-
}
3137+
return (string) $this->hash;
3138+
}
31443139

3140+
public function getHashInt(): int
3141+
{
31453142
return $this->hash;
31463143
}
31473144

@@ -3470,6 +3467,7 @@ public function __clone()
34703467
}
34713468
}
34723469
}
3470+
$this->hash = spl_object_id($this);
34733471
}
34743472

34753473
/**

‎src/PhpSpreadsheet/Writer/Xlsx/Style.php

+1
Original file line numberDiff line numberDiff line change
@@ -142,6 +142,7 @@ public function writeStyles(Spreadsheet $spreadsheet): string
142142

143143
// dxf
144144
for ($i = 0; $i < $this->getParentWriter()->getStylesConditionalHashTable()->count(); ++$i) {
145+
/** @var ?Conditional */
145146
$thisstyle = $this->getParentWriter()->getStylesConditionalHashTable()->getByIndex($i);
146147
if ($thisstyle !== null) {
147148
$this->writeCellStyleDxf($objWriter, $thisstyle->getStyle());

‎tests/PhpSpreadsheetTests/Shared/DateTest.php

+3-1
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
use DateTimeZone;
99
use PhpOffice\PhpSpreadsheet\Exception;
1010
use PhpOffice\PhpSpreadsheet\Shared\Date;
11+
use PhpOffice\PhpSpreadsheet\Spreadsheet;
1112
use PhpOffice\PhpSpreadsheet\Style\NumberFormat;
1213
use PHPUnit\Framework\TestCase;
1314

@@ -206,7 +207,7 @@ public function testVarious(): void
206207
$date = Date::PHPToExcel('2020-01-01');
207208
self::assertEquals(43831.0, $date);
208209

209-
$spreadsheet = new \PhpOffice\PhpSpreadsheet\Spreadsheet();
210+
$spreadsheet = new Spreadsheet();
210211
$sheet = $spreadsheet->getActiveSheet();
211212
$sheet->setCellValue('B1', 'x');
212213
/** @var float|int|string */
@@ -248,5 +249,6 @@ public function testVarious(): void
248249
->getNumberFormat()
249250
->setFormatCode('yyyy-mm-dd');
250251
self::assertFalse(Date::isDateTime($cella4));
252+
$spreadsheet->disconnectWorksheets();
251253
}
252254
}

‎tests/PhpSpreadsheetTests/Shared/StringHelperInvalidCharTest.php

+1
Original file line numberDiff line numberDiff line change
@@ -43,5 +43,6 @@ public function testInvalidChar(): void
4343
$sheet->getCell("A$row")->getValue()
4444
);
4545
}
46+
$spreadsheet->disconnectWorksheets();
4647
}
4748
}

‎tests/PhpSpreadsheetTests/Style/BorderRangeTest.php

+2
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,7 @@ public function testBorderRangeInAction(): void
6262
}
6363
}
6464
}
65+
$spreadsheet->disconnectWorksheets();
6566
}
6667

6768
public function testBorderRangeDirectly(): void
@@ -71,5 +72,6 @@ public function testBorderRangeDirectly(): void
7172
$sheet = $spreadsheet->getActiveSheet();
7273
$style = $sheet->getStyle('A1:C1')->getBorders()->getTop()->setBorderStyle(Border::BORDER_THIN);
7374
self::assertSame('A1:C1', $style->getSelectedCells(), 'getSelectedCells should not change after a style operation on a border range');
75+
$spreadsheet->disconnectWorksheets();
7476
}
7577
}

‎tests/PhpSpreadsheetTests/Style/BorderTest.php

+8
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ public function testAllBorders(): void
3131
self::assertSame(Border::BORDER_THIN, $borders->getRight()->getBorderStyle());
3232
self::assertSame(Border::BORDER_THIN, $borders->getLeft()->getBorderStyle());
3333
self::assertSame(Border::BORDER_NONE, $borders->getDiagonal()->getBorderStyle());
34+
$spreadsheet->disconnectWorksheets();
3435
}
3536

3637
public function testAllBordersArray(): void
@@ -45,6 +46,7 @@ public function testAllBordersArray(): void
4546
self::assertSame(Border::BORDER_THIN, $borders->getRight()->getBorderStyle());
4647
self::assertSame(Border::BORDER_THIN, $borders->getLeft()->getBorderStyle());
4748
self::assertSame(Border::BORDER_NONE, $borders->getDiagonal()->getBorderStyle());
49+
$spreadsheet->disconnectWorksheets();
4850
}
4951

5052
public function testAllBordersArrayNotSupervisor(): void
@@ -86,6 +88,7 @@ public function testOutline(): void
8688
self::assertSame(Border::BORDER_THIN, $sheet->getCell('B2')->getStyle()->getBorders()->getBottom()->getBorderStyle());
8789
self::assertSame(Border::BORDER_NONE, $sheet->getCell('B2')->getStyle()->getBorders()->getLeft()->getBorderStyle());
8890
self::assertSame(Border::BORDER_NONE, $sheet->getCell('B2')->getStyle()->getBorders()->getTop()->getBorderStyle());
91+
$spreadsheet->disconnectWorksheets();
8992
}
9093

9194
public function testInside(): void
@@ -115,6 +118,7 @@ public function testInside(): void
115118
self::assertSame(Border::BORDER_NONE, $sheet->getCell('B2')->getStyle()->getBorders()->getBottom()->getBorderStyle());
116119
self::assertSame(Border::BORDER_THIN, $sheet->getCell('B2')->getStyle()->getBorders()->getLeft()->getBorderStyle());
117120
self::assertSame(Border::BORDER_THIN, $sheet->getCell('B2')->getStyle()->getBorders()->getTop()->getBorderStyle());
121+
$spreadsheet->disconnectWorksheets();
118122
}
119123

120124
public function testHorizontal(): void
@@ -144,6 +148,7 @@ public function testHorizontal(): void
144148
self::assertSame(Border::BORDER_NONE, $sheet->getCell('B2')->getStyle()->getBorders()->getBottom()->getBorderStyle());
145149
self::assertSame(Border::BORDER_NONE, $sheet->getCell('B2')->getStyle()->getBorders()->getLeft()->getBorderStyle());
146150
self::assertSame(Border::BORDER_THIN, $sheet->getCell('B2')->getStyle()->getBorders()->getTop()->getBorderStyle());
151+
$spreadsheet->disconnectWorksheets();
147152
}
148153

149154
public function testVertical(): void
@@ -173,6 +178,7 @@ public function testVertical(): void
173178
self::assertSame(Border::BORDER_NONE, $sheet->getCell('B2')->getStyle()->getBorders()->getBottom()->getBorderStyle());
174179
self::assertSame(Border::BORDER_THIN, $sheet->getCell('B2')->getStyle()->getBorders()->getLeft()->getBorderStyle());
175180
self::assertSame(Border::BORDER_NONE, $sheet->getCell('B2')->getStyle()->getBorders()->getTop()->getBorderStyle());
181+
$spreadsheet->disconnectWorksheets();
176182
}
177183

178184
public function testNoSupervisorAllBorders(): void
@@ -233,6 +239,7 @@ public function testBorderStyle(): void
233239
$border->setBorderStyle(Border::BORDER_THIN)->setColor(new Color('FFFF0000'));
234240
self::assertEquals('FFFF0000', $border->getColor()->getARGB());
235241
self::assertEquals(Border::BORDER_THIN, $border->getBorderStyle());
242+
$spreadsheet->disconnectWorksheets();
236243
}
237244

238245
public function testDiagonalDirection(): void
@@ -245,5 +252,6 @@ public function testDiagonalDirection(): void
245252

246253
self::assertSame(Border::BORDER_MEDIUM, $borders->getDiagonal()->getBorderStyle());
247254
self::assertSame(Borders::DIAGONAL_BOTH, $borders->getDiagonalDirection());
255+
$spreadsheet->disconnectWorksheets();
248256
}
249257
}

‎tests/PhpSpreadsheetTests/Style/ConditionalFormatting/Wizard/WizardFactoryTest.php

+1
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,7 @@ public function testWizardFromConditional(string $sheetName, string $cellAddress
6666
$wizard = Wizard::fromConditional($conditional);
6767
self::assertEquals($expectedWizads[$index], $wizard::class);
6868
}
69+
$spreadsheet->disconnectWorksheets();
6970
}
7071

7172
public static function conditionalProvider(): array

0 commit comments

Comments
 (0)