Skip to content

Commit be65fbe

Browse files
oleibmanoleibman
authored
Change hash code for worksheet branch release1291 (#4306)
* Change hash code for worksheet Backport of [PR #4207](#4207) * Retitling Cloned Worksheets Backport of PR #4302 * Make Test Php7.4 Compatible
1 parent 51635f6 commit be65fbe

16 files changed

+257
-37
lines changed

‎.php-cs-fixer.dist.php

+1-1
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,7 @@
116116
'no_unneeded_final_method' => true,
117117
'no_unreachable_default_argument_value' => true,
118118
'no_unset_cast' => true,
119-
'no_unset_on_property' => true,
119+
'no_unset_on_property' => false,
120120
'no_unused_imports' => true,
121121
'no_useless_else' => true,
122122
'no_useless_return' => true,

‎CHANGELOG.md

+11
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,17 @@ 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 - 1.29.8
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+
819
# 2024-12-26 - 1.29.7
920

1021
### Deprecated

‎src/PhpSpreadsheet/ReferenceHelper.php

+2-2
Original file line numberDiff line numberDiff line change
@@ -913,7 +913,7 @@ private function updateNamedRange(DefinedName $definedName, Worksheet $worksheet
913913
{
914914
$cellAddress = $definedName->getValue();
915915
$asFormula = ($cellAddress[0] === '=');
916-
if ($definedName->getWorksheet() !== null && $definedName->getWorksheet()->getHashCode() === $worksheet->getHashCode()) {
916+
if ($definedName->getWorksheet() !== null && $definedName->getWorksheet()->getHashInt() === $worksheet->getHashInt()) {
917917
/**
918918
* If we delete the entire range that is referenced by a Named Range, MS Excel sets the value to #REF!
919919
* PhpSpreadsheet still only does a basic adjustment, so the Named Range will still reference Cells.
@@ -932,7 +932,7 @@ private function updateNamedRange(DefinedName $definedName, Worksheet $worksheet
932932

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

‎src/PhpSpreadsheet/Spreadsheet.php

+21-4
Original file line numberDiff line numberDiff line change
@@ -599,7 +599,7 @@ public function getActiveSheet()
599599
public function createSheet($sheetIndex = null)
600600
{
601601
$newSheet = new Worksheet($this);
602-
$this->addSheet($newSheet, $sheetIndex);
602+
$this->addSheet($newSheet, $sheetIndex, true);
603603

604604
return $newSheet;
605605
}
@@ -621,11 +621,24 @@ public function sheetNameExists($worksheetName)
621621
*
622622
* @param Worksheet $worksheet The worksheet to add
623623
* @param null|int $sheetIndex Index where sheet should go (0,1,..., or null for last)
624+
* @param bool $retitleIfNeeded add suffix if title exists in spreadsheet
624625
*
625626
* @return Worksheet
626627
*/
627-
public function addSheet(Worksheet $worksheet, $sheetIndex = null)
628+
public function addSheet(Worksheet $worksheet, $sheetIndex = null, $retitleIfNeeded = false)
628629
{
630+
if ($retitleIfNeeded) {
631+
$title = $worksheet->getTitle();
632+
if ($this->sheetNameExists($title)) {
633+
$i = 1;
634+
$newTitle = "$title $i";
635+
while ($this->sheetNameExists($newTitle)) {
636+
++$i;
637+
$newTitle = "$title $i";
638+
}
639+
$worksheet->setTitle($newTitle);
640+
}
641+
}
629642
if ($this->sheetNameExists($worksheet->getTitle())) {
630643
throw new Exception(
631644
"Workbook already contains a worksheet named '{$worksheet->getTitle()}'. Rename this worksheet first."
@@ -750,13 +763,17 @@ public function getSheetByNameOrThrow(string $worksheetName): Worksheet
750763
*
751764
* @return int index
752765
*/
753-
public function getIndex(Worksheet $worksheet)
766+
public function getIndex(Worksheet $worksheet, bool $noThrow = false)
754767
{
768+
$wsHash = $worksheet->getHashInt();
755769
foreach ($this->workSheetCollection as $key => $value) {
756-
if ($value->getHashCode() === $worksheet->getHashCode()) {
770+
if ($value->getHashInt() === $wsHash) {
757771
return $key;
758772
}
759773
}
774+
if ($noThrow) {
775+
return -1;
776+
}
760777

761778
throw new Exception('Sheet does not exist.');
762779
}

‎src/PhpSpreadsheet/Worksheet/BaseDrawing.php

+1-1
Original file line numberDiff line numberDiff line change
@@ -444,7 +444,7 @@ public function getHashCode()
444444
return md5(
445445
$this->name .
446446
$this->description .
447-
(($this->worksheet === null) ? '' : $this->worksheet->getHashCode()) .
447+
(($this->worksheet === null) ? '' : (string) $this->worksheet->getHashInt()) .
448448
$this->coordinates .
449449
$this->offsetX .
450450
$this->offsetY .

‎src/PhpSpreadsheet/Worksheet/Worksheet.php

+21-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;
@@ -31,7 +30,7 @@
3130
use PhpOffice\PhpSpreadsheet\Style\NumberFormat;
3231
use PhpOffice\PhpSpreadsheet\Style\Style;
3332

34-
class Worksheet implements IComparable
33+
class Worksheet
3534
{
3635
// Break types
3736
public const BREAK_NONE = 0;
@@ -338,17 +337,10 @@ class Worksheet implements IComparable
338337
*/
339338
private $tabColor;
340339

341-
/**
342-
* Dirty flag.
343-
*
344-
* @var bool
345-
*/
346-
private $dirty = true;
347-
348340
/**
349341
* Hash.
350342
*
351-
* @var string
343+
* @var int
352344
*/
353345
private $hash;
354346

@@ -368,6 +360,7 @@ public function __construct(?Spreadsheet $parent = null, $title = 'Worksheet')
368360
{
369361
// Set parent and title
370362
$this->parent = $parent;
363+
$this->hash = spl_object_id($this);
371364
$this->setTitle($title, false);
372365
// setTitle can change $pTitle
373366
$this->setCodeName($this->getTitle());
@@ -424,6 +417,12 @@ public function __destruct()
424417
$this->rowDimensions = [];
425418
}
426419

420+
public function __wakeup(): void
421+
{
422+
$this->hash = spl_object_id($this);
423+
$this->parent = null;
424+
}
425+
427426
/**
428427
* Return the cell collection.
429428
*
@@ -914,7 +913,7 @@ public function setTitle($title, $updateFormulaCellReferences = true, $validate
914913
// Syntax check
915914
self::checkSheetTitle($title);
916915

917-
if ($this->parent) {
916+
if ($this->parent && $this->parent->getIndex($this, true) >= 0) {
918917
// Is there already such sheet name?
919918
if ($this->parent->sheetNameExists($title)) {
920919
// Use name, but append with lowest possible integer
@@ -943,9 +942,8 @@ public function setTitle($title, $updateFormulaCellReferences = true, $validate
943942

944943
// Set title
945944
$this->title = $title;
946-
$this->dirty = true;
947945

948-
if ($this->parent && $this->parent->getCalculationEngine()) {
946+
if ($this->parent && $this->parent->getIndex($this, true) >= 0 && $this->parent->getCalculationEngine()) {
949947
// New title
950948
$newTitle = $this->getTitle();
951949
$this->parent->getCalculationEngine()
@@ -1088,7 +1086,6 @@ public function getProtection()
10881086
public function setProtection(Protection $protection)
10891087
{
10901088
$this->protection = $protection;
1091-
$this->dirty = true;
10921089

10931090
return $this;
10941091
}
@@ -3137,7 +3134,7 @@ private function validateNamedRange(string $definedName, bool $returnNullIfInval
31373134

31383135
if ($namedRange->getLocalOnly()) {
31393136
$worksheet = $namedRange->getWorksheet();
3140-
if ($worksheet === null || $this->getHashCode() !== $worksheet->getHashCode()) {
3137+
if ($worksheet === null || $this->getHashInt() !== $worksheet->getHashInt()) {
31413138
if ($returnNullIfInvalid) {
31423139
return null;
31433140
}
@@ -3278,17 +3275,20 @@ public function garbageCollect()
32783275
}
32793276

32803277
/**
3281-
* Get hash code.
3278+
* @deprecated 3.5.0 use getHashInt instead.
32823279
*
32833280
* @return string Hash code
32843281
*/
32853282
public function getHashCode()
32863283
{
3287-
if ($this->dirty) {
3288-
$this->hash = md5($this->title . $this->autoFilter . ($this->protection->isProtectionEnabled() ? 't' : 'f') . __CLASS__);
3289-
$this->dirty = false;
3290-
}
3284+
return (string) $this->hash;
3285+
}
32913286

3287+
/**
3288+
* @return int Hash code
3289+
*/
3290+
public function getHashInt()
3291+
{
32923292
return $this->hash;
32933293
}
32943294

@@ -3620,6 +3620,7 @@ public function __clone()
36203620
}
36213621
}
36223622
}
3623+
$this->hash = spl_object_id($this);
36233624
}
36243625

36253626
/**

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

+1
Original file line numberDiff line numberDiff line change
@@ -142,6 +142,7 @@ public function writeStyles(Spreadsheet $spreadsheet)
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
@@ -5,6 +5,7 @@
55
use DateTimeZone;
66
use PhpOffice\PhpSpreadsheet\Exception;
77
use PhpOffice\PhpSpreadsheet\Shared\Date;
8+
use PhpOffice\PhpSpreadsheet\Spreadsheet;
89
use PhpOffice\PhpSpreadsheet\Style\NumberFormat;
910
use PHPUnit\Framework\TestCase;
1011

@@ -230,7 +231,7 @@ public function testVarious(): void
230231
$date = Date::PHPToExcel('2020-01-01');
231232
self::assertEquals(43831.0, $date);
232233

233-
$spreadsheet = new \PhpOffice\PhpSpreadsheet\Spreadsheet();
234+
$spreadsheet = new Spreadsheet();
234235
$sheet = $spreadsheet->getActiveSheet();
235236
$sheet->setCellValue('B1', 'x');
236237
$val = $sheet->getCell('B1')->getValue();
@@ -271,5 +272,6 @@ public function testVarious(): void
271272
->getNumberFormat()
272273
->setFormatCode('yyyy-mm-dd');
273274
self::assertFalse(Date::isDateTime($cella4));
275+
$spreadsheet->disconnectWorksheets();
274276
}
275277
}

‎tests/PhpSpreadsheetTests/Shared/StringHelperInvalidCharTest.php

+1
Original file line numberDiff line numberDiff line change
@@ -40,5 +40,6 @@ public function testInvalidChar(): void
4040
$sheet->getCell("A$row")->getValue()
4141
);
4242
}
43+
$spreadsheet->disconnectWorksheets();
4344
}
4445
}

‎tests/PhpSpreadsheetTests/Style/BorderRangeTest.php

+2
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,7 @@ public function testBorderRangeInAction(): void
6060
}
6161
}
6262
}
63+
$spreadsheet->disconnectWorksheets();
6364
}
6465

6566
public function testBorderRangeDirectly(): void
@@ -69,5 +70,6 @@ public function testBorderRangeDirectly(): void
6970
$sheet = $spreadsheet->getActiveSheet();
7071
$style = $sheet->getStyle('A1:C1')->getBorders()->getTop()->setBorderStyle(Border::BORDER_THIN);
7172
self::assertSame('A1:C1', $style->getSelectedCells(), 'getSelectedCells should not change after a style operation on a border range');
73+
$spreadsheet->disconnectWorksheets();
7274
}
7375
}

‎tests/PhpSpreadsheetTests/Style/BorderTest.php

+9
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ public function testAllBorders(): void
2929
self::assertSame(Border::BORDER_THIN, $borders->getRight()->getBorderStyle());
3030
self::assertSame(Border::BORDER_THIN, $borders->getLeft()->getBorderStyle());
3131
self::assertSame(Border::BORDER_NONE, $borders->getDiagonal()->getBorderStyle());
32+
$spreadsheet->disconnectWorksheets();
3233
}
3334

3435
public function testAllBordersArray(): void
@@ -43,6 +44,7 @@ public function testAllBordersArray(): void
4344
self::assertSame(Border::BORDER_THIN, $borders->getRight()->getBorderStyle());
4445
self::assertSame(Border::BORDER_THIN, $borders->getLeft()->getBorderStyle());
4546
self::assertSame(Border::BORDER_NONE, $borders->getDiagonal()->getBorderStyle());
47+
$spreadsheet->disconnectWorksheets();
4648
}
4749

4850
public function testAllBordersArrayNotSupervisor(): void
@@ -84,6 +86,7 @@ public function testOutline(): void
8486
self::assertSame(Border::BORDER_THIN, $sheet->getCell('B2')->getStyle()->getBorders()->getBottom()->getBorderStyle());
8587
self::assertSame(Border::BORDER_NONE, $sheet->getCell('B2')->getStyle()->getBorders()->getLeft()->getBorderStyle());
8688
self::assertSame(Border::BORDER_NONE, $sheet->getCell('B2')->getStyle()->getBorders()->getTop()->getBorderStyle());
89+
$spreadsheet->disconnectWorksheets();
8790
}
8891

8992
public function testInside(): void
@@ -113,6 +116,7 @@ public function testInside(): void
113116
self::assertSame(Border::BORDER_NONE, $sheet->getCell('B2')->getStyle()->getBorders()->getBottom()->getBorderStyle());
114117
self::assertSame(Border::BORDER_THIN, $sheet->getCell('B2')->getStyle()->getBorders()->getLeft()->getBorderStyle());
115118
self::assertSame(Border::BORDER_THIN, $sheet->getCell('B2')->getStyle()->getBorders()->getTop()->getBorderStyle());
119+
$spreadsheet->disconnectWorksheets();
116120
}
117121

118122
public function testHorizontal(): void
@@ -142,6 +146,7 @@ public function testHorizontal(): void
142146
self::assertSame(Border::BORDER_NONE, $sheet->getCell('B2')->getStyle()->getBorders()->getBottom()->getBorderStyle());
143147
self::assertSame(Border::BORDER_NONE, $sheet->getCell('B2')->getStyle()->getBorders()->getLeft()->getBorderStyle());
144148
self::assertSame(Border::BORDER_THIN, $sheet->getCell('B2')->getStyle()->getBorders()->getTop()->getBorderStyle());
149+
$spreadsheet->disconnectWorksheets();
145150
}
146151

147152
public function testVertical(): void
@@ -171,6 +176,7 @@ public function testVertical(): void
171176
self::assertSame(Border::BORDER_NONE, $sheet->getCell('B2')->getStyle()->getBorders()->getBottom()->getBorderStyle());
172177
self::assertSame(Border::BORDER_THIN, $sheet->getCell('B2')->getStyle()->getBorders()->getLeft()->getBorderStyle());
173178
self::assertSame(Border::BORDER_NONE, $sheet->getCell('B2')->getStyle()->getBorders()->getTop()->getBorderStyle());
179+
$spreadsheet->disconnectWorksheets();
174180
}
175181

176182
public function testNoSupervisorAllBorders(): void
@@ -215,6 +221,7 @@ public function testGetSharedComponentPseudo(): void
215221
$sheet = $spreadsheet->getActiveSheet();
216222
$sheet->getStyle('A1')->getBorders()->getHorizontal()->setBorderStyle(Border::BORDER_MEDIUM);
217223
$sheet->getStyle('A1')->getBorders()->getHorizontal()->getSharedComponent();
224+
$spreadsheet->disconnectWorksheets();
218225
}
219226

220227
public function testBorderStyle(): void
@@ -231,6 +238,7 @@ public function testBorderStyle(): void
231238
$border->setBorderStyle(Border::BORDER_THIN)->setColor(new Color('FFFF0000'));
232239
self::assertEquals('FFFF0000', $border->getColor()->getARGB());
233240
self::assertEquals(Border::BORDER_THIN, $border->getBorderStyle());
241+
$spreadsheet->disconnectWorksheets();
234242
}
235243

236244
public function testDiagonalDirection(): void
@@ -243,5 +251,6 @@ public function testDiagonalDirection(): void
243251

244252
self::assertSame(Border::BORDER_MEDIUM, $borders->getDiagonal()->getBorderStyle());
245253
self::assertSame(Borders::DIAGONAL_BOTH, $borders->getDiagonalDirection());
254+
$spreadsheet->disconnectWorksheets();
246255
}
247256
}

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

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

7273
public static function conditionalProvider(): array

0 commit comments

Comments
 (0)