Register style can be skipped when already registered

src/Spout/Common/Entity/Style/Style.php

@@ -84,6 +84,9 @@ class Style
     /** @var bool */
     private $hasSetFormat = false;
 
+    /** @var bool */
+    private $isRegistered = false;
+
     /**
      * @return int|null
      */
@@ -463,4 +466,23 @@ class Style
     {
         return $this->hasSetFormat;
     }
+
+    public function setRegistered(bool $isRegistered = true) : void
+    {
+        $this->isRegistered = $isRegistered;
+    }
+
+    /**
+     * @return bool
+     */
+    public function isRegistered() : bool
+    {
+        return $this->isRegistered;
+    }
+
+    public function register(int $id) : void
+    {
+        $this->setId($id);
+        $this->isRegistered = true;
+    }
 }

src/Spout/Writer/Common/Manager/Style/StyleManager.php

@@ -50,13 +50,11 @@ class StyleManager implements StyleManagerInterface
      * Typically, set "wrap text" if a cell contains a new line.
      *
      * @param Cell $cell
-     * @return Style
+     * @return Style|null The eventually updated style
      */
-    public function applyExtraStylesIfNeeded(Cell $cell)
+    public function applyExtraStylesIfNeeded(Cell $cell) : ?Style
     {
-        $updatedStyle = $this->applyWrapTextIfCellContainsNewLine($cell);
+        return $this->applyWrapTextIfCellContainsNewLine($cell);
-
-        return $updatedStyle;
     }
 
     /**
@@ -69,21 +67,23 @@ class StyleManager implements StyleManagerInterface
      *        on the Windows version of Excel...
      *
      * @param Cell $cell The cell the style should be applied to
-     * @return \Box\Spout\Common\Entity\Style\Style The eventually updated style
+     * @return Style|null The eventually updated style
      */
-    protected function applyWrapTextIfCellContainsNewLine(Cell $cell)
+    protected function applyWrapTextIfCellContainsNewLine(Cell $cell) : ?Style
     {
         $cellStyle = $cell->getStyle();
 
         // if the "wrap text" option is already set, no-op
         if ($cellStyle->hasSetWrapText()) {
-            return $cellStyle;
+            return null;
         }
 
         if ($cell->isString() && \strpos($cell->getValue(), "\n") !== false) {
             $cellStyle->setShouldWrapText();
+
+            return $cellStyle;
         }
 
-        return $cellStyle;
+        return null;
     }
 }

src/Spout/Writer/Common/Manager/Style/StyleManagerInterface.php

@@ -24,7 +24,7 @@ interface StyleManagerInterface
      * Typically, set "wrap text" if a cell contains a new line.
      *
      * @param Cell $cell
-     * @return Style The updated style
+     * @return Style|null The eventually updated style
      */
-    public function applyExtraStylesIfNeeded(Cell $cell);
+    public function applyExtraStylesIfNeeded(Cell $cell) : ?Style;
 }

src/Spout/Writer/Common/Manager/Style/StyleRegistry.php

@@ -38,7 +38,7 @@ class StyleRegistry
 
         if (!$this->hasSerializedStyleAlreadyBeenRegistered($serializedStyle)) {
             $nextStyleId = \count($this->serializedStyleToStyleIdMappingTable);
-            $style->setId($nextStyleId);
+            $style->register($nextStyleId);
 
             $this->serializedStyleToStyleIdMappingTable[$serializedStyle] = $nextStyleId;
             $this->styleIdToStyleMappingTable[$nextStyleId] = $style;
@@ -112,9 +112,10 @@ class StyleRegistry
      */
     public function serialize(Style $style)
     {
-        // In order to be able to properly compare style, set static ID value
+        // In order to be able to properly compare style, set static ID value and reset registration
         $currentId = $style->getId();
         $style->setId(0);
+        $style->setRegistered(false);
 
         $serializedStyle = \serialize($style);
 

src/Spout/Writer/ODS/Manager/Style/StyleRegistry.php

@@ -22,6 +22,10 @@ class StyleRegistry extends \Box\Spout\Writer\Common\Manager\Style\StyleRegistry
      */
     public function registerStyle(Style $style)
     {
+        if ($style->isRegistered()) {
+            return $style;
+        }
+
         $registeredStyle = parent::registerStyle($style);
         $this->usedFontsSet[$style->getFontName()] = true;
 

src/Spout/Writer/ODS/Manager/WorksheetManager.php

@@ -156,18 +156,26 @@ class WorksheetManager implements WorksheetManagerInterface
      * @throws InvalidArgumentException If a cell value's type is not supported
      * @return string
      */
-    private function applyStyleAndGetCellXML(Cell $cell, Style $rowStyle, $currentCellIndex, $nextCellIndex)
+    private function applyStyleAndGetCellXML(Cell $cell, Style &$rowStyle, $currentCellIndex, $nextCellIndex)
     {
         if ($cell->getStyle() instanceof EmptyStyle) {
             $cell->setStyle($rowStyle);
+
+            $extraStyle = $this->styleManager->applyExtraStylesIfNeeded($cell);
+
+            if ($extraStyle) {
+                $registeredStyle = $this->styleManager->registerStyle($extraStyle);
+            } else {
+                $registeredStyle = $rowStyle = $this->styleManager->registerStyle($rowStyle);
+            }
         } else {
             $mergedCellAndRowStyle = $this->styleMerger->merge($cell->getStyle(), $rowStyle);
             $cell->setStyle($mergedCellAndRowStyle);
+
+            $newCellStyle = $this->styleManager->applyExtraStylesIfNeeded($cell) ?: $mergedCellAndRowStyle;
+            $registeredStyle = $this->styleManager->registerStyle($newCellStyle);
         }
 
-        $newCellStyle = $this->styleManager->applyExtraStylesIfNeeded($cell);
-
-        $registeredStyle = $this->styleManager->registerStyle($newCellStyle);
         $styleIndex = $registeredStyle->getId() + 1; // 1-based
 
         $numTimesValueRepeated = ($nextCellIndex - $currentCellIndex);

src/Spout/Writer/XLSX/Manager/Style/StyleRegistry.php

@@ -119,6 +119,10 @@ class StyleRegistry extends \Box\Spout\Writer\Common\Manager\Style\StyleRegistry
      */
     public function registerStyle(Style $style)
     {
+        if ($style->isRegistered()) {
+            return $style;
+        }
+
         $registeredStyle = parent::registerStyle($style);
         $this->registerFill($registeredStyle);
         $this->registerFormat($registeredStyle);

src/Spout/Writer/XLSX/Manager/WorksheetManager.php

@@ -184,17 +184,25 @@ EOD;
      * @throws InvalidArgumentException If the given value cannot be processed
      * @return string
      */
-    private function applyStyleAndGetCellXML(Cell $cell, Style $rowStyle, $rowIndexOneBased, $columnIndexZeroBased)
+    private function applyStyleAndGetCellXML(Cell $cell, Style &$rowStyle, $rowIndexOneBased, $columnIndexZeroBased)
     {
         if ($cell->getStyle() instanceof EmptyStyle) {
             $cell->setStyle($rowStyle);
+
+            $extraStyle = $this->styleManager->applyExtraStylesIfNeeded($cell);
+
+            if ($extraStyle) {
+                $registeredStyle = $this->styleManager->registerStyle($extraStyle);
+            } else {
+                $registeredStyle = $rowStyle = $this->styleManager->registerStyle($rowStyle);
+            }
         } else {
             $mergedCellAndRowStyle = $this->styleMerger->merge($cell->getStyle(), $rowStyle);
             $cell->setStyle($mergedCellAndRowStyle);
-        }
 
-        $newCellStyle = $this->styleManager->applyExtraStylesIfNeeded($cell);
+            $newCellStyle = $this->styleManager->applyExtraStylesIfNeeded($cell) ?: $mergedCellAndRowStyle;
-        $registeredStyle = $this->styleManager->registerStyle($newCellStyle);
+            $registeredStyle = $this->styleManager->registerStyle($newCellStyle);
+        }
 
         return $this->getCellXML($rowIndexOneBased, $columnIndexZeroBased, $cell, $registeredStyle->getId());
     }

tests/Spout/Writer/Common/Manager/Style/StyleManagerTest.php

@@ -14,7 +14,7 @@ class StyleManagerTest extends TestCase
     /**
      * @return StyleManager
      */
-    private function getStyleManager()
+    private function getStyleManager() : StyleManager
     {
         $style = (new StyleBuilder())->build();
         $styleRegistry = new StyleRegistry($style);
@@ -22,10 +22,7 @@ class StyleManagerTest extends TestCase
         return new StyleManager($styleRegistry);
     }
 
-    /**
+    public function testApplyExtraStylesIfNeededShouldApplyWrapTextIfCellContainsNewLine() : void
-     * @return void
-     */
-    public function testApplyExtraStylesIfNeededShouldApplyWrapTextIfCellContainsNewLine()
     {
         $style = (new StyleBuilder())->build();
         $this->assertFalse($style->shouldWrapText());
@@ -33,13 +30,22 @@ class StyleManagerTest extends TestCase
         $styleManager = $this->getStyleManager();
         $updatedStyle = $styleManager->applyExtraStylesIfNeeded(new Cell("multi\nlines", $style));
 
+        $this->assertNotNull($updatedStyle);
         $this->assertTrue($updatedStyle->shouldWrapText());
     }
 
-    /**
+    public function testApplyExtraStylesIfNeededShouldReturnNullIfWrapTextNotNeeded() : void
-     * @return void
+    {
-     */
+        $style = (new StyleBuilder())->build();
-    public function testApplyExtraStylesIfNeededShouldDoNothingIfWrapTextAlreadyApplied()
+        $this->assertFalse($style->shouldWrapText());
+
+        $styleManager = $this->getStyleManager();
+        $updatedStyle = $styleManager->applyExtraStylesIfNeeded(new Cell('oneline', $style));
+
+        $this->assertNull($updatedStyle);
+    }
+
+    public function testApplyExtraStylesIfNeededShouldReturnNullIfWrapTextAlreadyApplied() : void
     {
         $style = (new StyleBuilder())->setShouldWrapText()->build();
         $this->assertTrue($style->shouldWrapText());
@@ -47,6 +53,6 @@ class StyleManagerTest extends TestCase
         $styleManager = $this->getStyleManager();
         $updatedStyle = $styleManager->applyExtraStylesIfNeeded(new Cell("multi\nlines", $style));
 
-        $this->assertTrue($updatedStyle->shouldWrapText());
+        $this->assertNull($updatedStyle);
     }
 }