Enforce sheet name uniqueness per workbook (#397)

Instead of across all workbooks (in case of multiple spreadsheets being created at the same time).

src/Spout/Writer/Common/Internal/AbstractWorkbook.php

@@ -16,6 +16,9 @@ abstract class AbstractWorkbook implements WorkbookInterface
     /** @var bool Whether new sheets should be automatically created when the max rows limit per sheet is reached */
     protected $shouldCreateNewSheetsAutomatically;
 
+    /** @var string Timestamp based unique ID identifying the workbook */
+    protected $internalId;
+
     /** @var WorksheetInterface[] Array containing the workbook's sheets */
     protected $worksheets = [];
 
@@ -30,6 +33,7 @@ abstract class AbstractWorkbook implements WorkbookInterface
     public function __construct($shouldCreateNewSheetsAutomatically, $defaultRowStyle)
     {
         $this->shouldCreateNewSheetsAutomatically = $shouldCreateNewSheetsAutomatically;
+        $this->internalId = uniqid();
     }
 
     /**

src/Spout/Writer/Common/Sheet.php

@@ -7,7 +7,7 @@ use Box\Spout\Writer\Exception\InvalidSheetNameException;
 
 /**
  * Class Sheet
- * External representation of a worksheet within a ODS file
+ * External representation of a worksheet
  *
  * @package Box\Spout\Writer\Common
  */
@@ -21,12 +21,15 @@ class Sheet
     /** @var array Invalid characters that cannot be contained in the sheet name */
     private static $INVALID_CHARACTERS_IN_SHEET_NAME = ['\\', '/', '?', '*', ':', '[', ']'];
 
-    /** @var array Associative array [SHEET_INDEX] => [SHEET_NAME] keeping track of sheets' name to enforce uniqueness */
+    /** @var array Associative array [WORKBOOK_ID] => [[SHEET_INDEX] => [SHEET_NAME]] keeping track of sheets' name to enforce uniqueness per workbook */
     protected static $SHEETS_NAME_USED = [];
 
     /** @var int Index of the sheet, based on order in the workbook (zero-based) */
     protected $index;
 
+    /** @var string ID of the sheet's associated workbook. Used to restrict sheet name uniqueness enforcement to a single workbook */
+    protected $associatedWorkbookId;
+
     /** @var string Name of the sheet */
     protected $name;
 
@@ -35,10 +38,16 @@ class Sheet
 
     /**
      * @param int $sheetIndex Index of the sheet, based on order in the workbook (zero-based)
+     * @param string $associatedWorkbookId ID of the sheet's associated workbook
      */
-    public function __construct($sheetIndex)
+    public function __construct($sheetIndex, $associatedWorkbookId)
     {
         $this->index = $sheetIndex;
+        $this->associatedWorkbookId = $associatedWorkbookId;
+        if (!isset(self::$SHEETS_NAME_USED[$associatedWorkbookId])) {
+            self::$SHEETS_NAME_USED[$associatedWorkbookId] = [];
+        }
+
         $this->stringHelper = new StringHelper();
         $this->setName(self::DEFAULT_SHEET_NAME_PREFIX . ($sheetIndex + 1));
     }
@@ -78,7 +87,7 @@ class Sheet
         $this->throwIfNameIsInvalid($name);
 
         $this->name = $name;
-        self::$SHEETS_NAME_USED[$this->index] = $name;
+        self::$SHEETS_NAME_USED[$this->associatedWorkbookId][$this->index] = $name;
 
         return $this;
     }
@@ -163,7 +172,7 @@ class Sheet
      */
     protected function isNameUnique($name)
     {
-        foreach (self::$SHEETS_NAME_USED as $sheetIndex => $sheetName) {
+        foreach (self::$SHEETS_NAME_USED[$this->associatedWorkbookId] as $sheetIndex => $sheetName) {
             if ($sheetIndex !== $this->index && $sheetName === $name) {
                 return false;
             }

src/Spout/Writer/ODS/Internal/Workbook.php

@@ -69,7 +69,7 @@ class Workbook extends AbstractWorkbook
     public function addNewSheet()
     {
         $newSheetIndex = count($this->worksheets);
-        $sheet = new Sheet($newSheetIndex);
+        $sheet = new Sheet($newSheetIndex, $this->internalId);
 
         $sheetsContentTempFolder = $this->fileSystemHelper->getSheetsContentTempFolder();
         $worksheet = new Worksheet($sheet, $sheetsContentTempFolder);

src/Spout/Writer/XLSX/Internal/Workbook.php

@@ -83,7 +83,7 @@ class Workbook extends AbstractWorkbook
     public function addNewSheet()
     {
         $newSheetIndex = count($this->worksheets);
-        $sheet = new Sheet($newSheetIndex);
+        $sheet = new Sheet($newSheetIndex, $this->internalId);
 
         $worksheetFilesFolder = $this->fileSystemHelper->getXlWorksheetsFolder();
         $worksheet = new Worksheet($sheet, $worksheetFilesFolder, $this->sharedStringsHelper, $this->styleHelper, $this->shouldUseInlineStrings);

tests/Spout/Writer/Common/SheetTest.php

@@ -14,7 +14,7 @@ class SheetTest extends \PHPUnit_Framework_TestCase
      */
     public function testGetSheetName()
     {
-        $sheets = [new Sheet(0), new Sheet(1)];
+        $sheets = [new Sheet(0, 'workbookId1'), new Sheet(1, 'workbookId1')];
 
         $this->assertEquals('Sheet1', $sheets[0]->getName(), 'Invalid name for the first sheet');
         $this->assertEquals('Sheet2', $sheets[1]->getName(), 'Invalid name for the second sheet');
@@ -26,7 +26,7 @@ class SheetTest extends \PHPUnit_Framework_TestCase
     public function testSetSheetNameShouldCreateSheetWithCustomName()
     {
         $customSheetName = 'CustomName';
-        $sheet = new Sheet(0);
+        $sheet = new Sheet(0, 'workbookId1');
         $sheet->setName($customSheetName);
 
         $this->assertEquals($customSheetName, $sheet->getName(), "The sheet name should have been changed to '$customSheetName'");
@@ -63,7 +63,7 @@ class SheetTest extends \PHPUnit_Framework_TestCase
      */
     public function testSetSheetNameShouldThrowOnInvalidName($customSheetName)
     {
-        (new Sheet(0))->setName($customSheetName);
+        (new Sheet(0, 'workbookId1'))->setName($customSheetName);
     }
 
     /**
@@ -72,7 +72,7 @@ class SheetTest extends \PHPUnit_Framework_TestCase
     public function testSetSheetNameShouldNotThrowWhenSettingSameNameAsCurrentOne()
     {
         $customSheetName = 'Sheet name';
-        $sheet = new Sheet(0);
+        $sheet = new Sheet(0, 'workbookId1');
         $sheet->setName($customSheetName);
         $sheet->setName($customSheetName);
     }
@@ -85,10 +85,27 @@ class SheetTest extends \PHPUnit_Framework_TestCase
     {
         $customSheetName = 'Sheet name';
 
-        $sheet = new Sheet(0);
+        $sheet = new Sheet(0, 'workbookId1');
         $sheet->setName($customSheetName);
 
-        $sheet = new Sheet(1);
+        $sheet = new Sheet(1, 'workbookId1');
+        $sheet->setName($customSheetName);
+    }
+
+    /**
+     * @return void
+     */
+    public function testSetSheetNameShouldNotThrowWhenSameNameUsedInDifferentWorkbooks()
+    {
+        $customSheetName = 'Sheet name';
+
+        $sheet = new Sheet(0, 'workbookId1');
+        $sheet->setName($customSheetName);
+
+        $sheet = new Sheet(0, 'workbookId2');
+        $sheet->setName($customSheetName);
+
+        $sheet = new Sheet(1, 'workbookId3');
         $sheet->setName($customSheetName);
     }
 }