Merge pull request #4153 from oleibman/issue4128b

Improve Xlsx Reader Speed
PHPOffice · Sep 5, 2024 · 50ec30b · 50ec30b
2 parents f023e0d + bba5719
commit 50ec30b
Show file tree

Hide file tree

Showing 5 changed files with 96 additions and 6 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -41,6 +41,7 @@ and this project adheres to [Semantic Versioning](https://semver.org).
 
 - Xls Reader Some Ranges Not Handled Properly. [Issue #1570](https://github.com/PHPOffice/PhpSpreadsheet/issues/1570) [PR #4140](https://github.com/PHPOffice/PhpSpreadsheet/pull/4140)
 - Better Handling of legacyDrawing Xml. [Issue #4105](https://github.com/PHPOffice/PhpSpreadsheet/issues/4105) [PR #4122](https://github.com/PHPOffice/PhpSpreadsheet/pull/4122)
+- Improve Xlsx Reader Speed. [Issue #3917](https://github.com/PHPOffice/PhpSpreadsheet/issues/3917) [PR #4153](https://github.com/PHPOffice/PhpSpreadsheet/pull/4153)
 
 ## 2024-08-07 - 2.2.2
 

diff --git a/src/PhpSpreadsheet/Cell/Cell.php b/src/PhpSpreadsheet/Cell/Cell.php
@@ -318,7 +318,21 @@ public function setValueExplicit(mixed $value, string $dataType = DataType::TYPE
         $this->updateInCollection();
         $cellCoordinate = $this->getCoordinate();
         self::updateIfCellIsTableHeader($this->getParent()?->getParent(), $this, $oldValue, $value);
-        $this->getWorksheet()->applyStylesFromArray($cellCoordinate, ['quotePrefix' => $quotePrefix]);
+        $worksheet = $this->getWorksheet();
+        $spreadsheet = $worksheet->getParent();
+        if (isset($spreadsheet)) {
+            $originalSelected = $worksheet->getSelectedCells();
+            $activeSheetIndex = $spreadsheet->getActiveSheetIndex();
+            $style = $this->getStyle();
+            $oldQuotePrefix = $style->getQuotePrefix();
+            if ($oldQuotePrefix !== $quotePrefix) {
+                $style->setQuotePrefix($quotePrefix);
+            }
+            $worksheet->setSelectedCells($originalSelected);
+            if ($activeSheetIndex >= 0) {
+                $spreadsheet->setActiveSheetIndex($activeSheetIndex);
+            }
+        }
 
         return $this->getParent()?->get($cellCoordinate) ?? $this;
     }

diff --git a/src/PhpSpreadsheet/Reader/Xlsx.php b/src/PhpSpreadsheet/Reader/Xlsx.php
@@ -950,16 +950,16 @@ protected function loadSpreadsheetFromFile(string $filename): Spreadsheet
 
                                             // Style information?
                                             if (!$this->readDataOnly) {
-                                                $holdSelected = $docSheet->getSelectedCells();
                                                 $cAttrS = (int) ($cAttr['s'] ?? 0);
                                                 // no style index means 0, it seems
                                                 $cAttrS = isset($styles[$cAttrS]) ? $cAttrS : 0;
                                                 $cell->setXfIndex($cAttrS);
                                                 // issue 3495
                                                 if ($cellDataType === DataType::TYPE_FORMULA && $styles[$cAttrS]->quotePrefix === true) {
+                                                    $holdSelected = $docSheet->getSelectedCells();
                                                     $cell->getStyle()->setQuotePrefix(false);
+                                                    $docSheet->setSelectedCells($holdSelected);
                                                 }
-                                                $docSheet->setSelectedCells($holdSelected);
                                             }
                                         }
                                         ++$rowIndex;

diff --git a/src/PhpSpreadsheet/Worksheet/Worksheet.php b/src/PhpSpreadsheet/Worksheet/Worksheet.php
@@ -3730,10 +3730,8 @@ public function applyStylesFromArray(string $coordinate, array $styleArray): boo
         }
         $activeSheetIndex = $spreadsheet->getActiveSheetIndex();
         $originalSelected = $this->selectedCells;
-        $originalActive = $this->activeCell;
         $this->getStyle($coordinate)->applyFromArray($styleArray);
-        $this->activeCell = $originalActive;
-        $this->selectedCells = $originalSelected;
+        $this->setSelectedCells($originalSelected);
         if ($activeSheetIndex >= 0) {
             $spreadsheet->setActiveSheetIndex($activeSheetIndex);
         }

diff --git a/tests/PhpSpreadsheetTests/Worksheet/ApplyStylesTest.php b/tests/PhpSpreadsheetTests/Worksheet/ApplyStylesTest.php
@@ -0,0 +1,77 @@
+<?php
+
+declare(strict_types=1);
+
+namespace PhpOffice\PhpSpreadsheetTests\Worksheet;
+
+use PhpOffice\PhpSpreadsheet\Spreadsheet;
+use PhpOffice\PhpSpreadsheet\Worksheet\Worksheet;
+use PHPUnit\Framework\TestCase;
+
+class ApplyStylesTest extends TestCase
+{
+    public function testApplyFromArray(): void
+    {
+        $spreadsheet = new Spreadsheet();
+        $sheet1 = $spreadsheet->getActiveSheet();
+        $sheet2 = $spreadsheet->createSheet();
+        $sheet3 = $spreadsheet->createSheet();
+        $cell = 'B4';
+        $sheet1->getCell($cell)->setValue('first');
+        $sheet1->getStyle($cell)->getFont()->setName('Arial');
+        $cell = 'C9';
+        $sheet2->getCell($cell)->setValue('second');
+        $sheet2->getStyle($cell)->getFont()->setName('Arial');
+        $cell = 'A6';
+        $sheet3->getCell($cell)->setValue('third');
+        $sheet3->getStyle($cell)->getFont()->setName('Arial');
+        self::assertSame(2, $spreadsheet->getActiveSheetIndex());
+        self::assertSame('B4', $sheet1->getSelectedCells());
+        self::assertSame('C9', $sheet2->getSelectedCells());
+        self::assertSame('A6', $sheet3->getSelectedCells());
+        $cell = 'D12';
+        $styleArray = ['font' => ['name' => 'Courier New']];
+        $sheet2->getStyle($cell)->applyFromArray($styleArray);
+        self::assertSame(1, $spreadsheet->getActiveSheetIndex());
+        self::assertSame('B4', $sheet1->getSelectedCells());
+        self::assertSame('D12', $sheet2->getSelectedCells());
+        self::assertSame('A6', $sheet3->getSelectedCells());
+        $spreadsheet->disconnectWorksheets();
+    }
+
+    public function testApplyStylesFromArray(): void
+    {
+        $spreadsheet = new Spreadsheet();
+        $sheet1 = $spreadsheet->getActiveSheet();
+        $sheet2 = $spreadsheet->createSheet();
+        $sheet3 = $spreadsheet->createSheet();
+        $cell = 'B4';
+        $sheet1->getCell($cell)->setValue('first');
+        $sheet1->getStyle($cell)->getFont()->setName('Arial');
+        $cell = 'C9';
+        $sheet2->getCell($cell)->setValue('second');
+        $sheet2->getStyle($cell)->getFont()->setName('Arial');
+        $cell = 'A6';
+        $sheet3->getCell($cell)->setValue('third');
+        $sheet3->getStyle($cell)->getFont()->setName('Arial');
+        self::assertSame(2, $spreadsheet->getActiveSheetIndex());
+        self::assertSame('B4', $sheet1->getSelectedCells());
+        self::assertSame('C9', $sheet2->getSelectedCells());
+        self::assertSame('A6', $sheet3->getSelectedCells());
+        $cell = 'D12';
+        $styleArray = ['font' => ['name' => 'Courier New']];
+        $sheet2->applyStylesFromArray($cell, $styleArray);
+        self::assertSame(2, $spreadsheet->getActiveSheetIndex(), 'should be unchanged');
+        self::assertSame('B4', $sheet1->getSelectedCells(), 'should be unchanged');
+        self::assertSame('C9', $sheet2->getSelectedCells(), 'should be unchanged');
+        self::assertSame('A6', $sheet3->getSelectedCells(), 'should be unchanged');
+        $spreadsheet->disconnectWorksheets();
+    }
+
+    public function testNoSpreadsheet(): void
+    {
+        $sheet2 = new Worksheet();
+        $cell = 'D12';
+        self::assertFalse($sheet2->applyStylesFromArray($cell, ['font' => ['name' => 'Courier New']]));
+    }
+}