Prevent error when close() called while writer already closed (#402)

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

README.md

@@ -200,16 +200,16 @@ $writer->setDefaultRowStyle($defaultStyle)
 
 Unfortunately, Spout does not support all the possible formatting options yet. But you can find the most important ones:
 
-Category  | Property      | API
-----------|---------------|---------------------------------------
-Font      | Bold          | `StyleBuilder::setFontBold()`
-          | Italic        | `StyleBuilder::setFontItalic()`
-          | Underline     | `StyleBuilder::setFontUnderline()`
-          | Strikethrough | `StyleBuilder::setFontStrikethrough()`
-          | Font name     | `StyleBuilder::setFontName('Arial')`
-          | Font size     | `StyleBuilder::setFontSize(14)`
-          | Font color    | `StyleBuilder::setFontColor(Color::BLUE)`<br>`StyleBuilder::setFontColor(Color::rgb(0, 128, 255))`
-Alignment | Wrap text     | `StyleBuilder::setShouldWrapText(true|false)`
+| Category  | Property      | API
+|-----------|---------------|---------------------------------------
+| Font      | Bold          | `StyleBuilder::setFontBold()`
+|           | Italic        | `StyleBuilder::setFontItalic()`
+|           | Underline     | `StyleBuilder::setFontUnderline()`
+|           | Strikethrough | `StyleBuilder::setFontStrikethrough()`
+|           | Font name     | `StyleBuilder::setFontName('Arial')`
+|           | Font size     | `StyleBuilder::setFontSize(14)`
+|           | Font color    | `StyleBuilder::setFontColor(Color::BLUE)`<br>`StyleBuilder::setFontColor(Color::rgb(0, 128, 255))`
+| Alignment | Wrap text     | `StyleBuilder::setShouldWrapText(true|false)`
 
 #### New sheet creation
 

src/Spout/Common/Escaper/ODS.php

@@ -14,31 +14,6 @@ class ODS implements EscaperInterface
 {
     use Singleton;
 
-    /** @var string Regex pattern to detect control characters that need to be escaped */
-    protected $escapableControlCharactersPattern;
-
-    /**
-     * Initializes the singleton instance
-     */
-    protected function init()
-    {
-        $this->escapableControlCharactersPattern = $this->getEscapableControlCharactersPattern();
-    }
-
-    /**
-     * @return string Regex pattern containing all escapable control characters
-     */
-    protected function getEscapableControlCharactersPattern()
-    {
-        // control characters values are from 0 to 1F (hex values) in the ASCII table
-        // some characters should not be escaped though: "\t", "\r" and "\n".
-        return '[\x00-\x08' .
-        // skipping "\t" (0x9) and "\n" (0xA)
-        '\x0B-\x0C' .
-        // skipping "\r" (0xD)
-        '\x0E-\x1F]';
-    }
-
     /**
      * Escapes the given string to make it compatible with XLSX
      *
@@ -49,15 +24,24 @@ class ODS implements EscaperInterface
     {
         if (defined('ENT_DISALLOWED')) {
             // 'ENT_DISALLOWED' ensures that invalid characters in the given document type are replaced.
-            // Otherwise characters like a vertical tab "\v" will make the XML document unreadable by the XML processor
+            // Otherwise control characters like a vertical tab "\v" will make the XML document unreadable by the XML processor
             // @link https://github.com/box/spout/issues/329
-            return htmlspecialchars($string, ENT_QUOTES | ENT_DISALLOWED);
+            $replacedString = htmlspecialchars($string, ENT_QUOTES | ENT_DISALLOWED);
         } else {
             // We are on hhvm or any other engine that does not support ENT_DISALLOWED
             $escapedString =  htmlspecialchars($string, ENT_QUOTES);
-            $replacedString = preg_replace('/'.$this->escapableControlCharactersPattern.'/', '�', $escapedString);
-            return $replacedString;
+
+            // control characters values are from 0 to 1F (hex values) in the ASCII table
+            // some characters should not be escaped though: "\t", "\r" and "\n".
+            $regexPattern = '[\x00-\x08' .
+                            // skipping "\t" (0x9) and "\n" (0xA)
+                            '\x0B-\x0C' .
+                            // skipping "\r" (0xD)
+                            '\x0E-\x1F]';
+            $replacedString = preg_replace("/$regexPattern/", '�', $escapedString);
         }
+
+        return $replacedString;
     }
 
     /**

src/Spout/Reader/CSV/RowIterator.php

@@ -14,10 +14,9 @@ use Box\Spout\Common\Helper\EncodingHelper;
 class RowIterator implements IteratorInterface
 {
     /**
-     * If no value is given to fgetcsv(), it defaults to 8192 (which may be too low).
-     * Alignement with other functions like fgets() is discussed here: https://bugs.php.net/bug.php?id=48421
+     * Value passed to fgetcsv. 0 means "unlimited" (slightly slower but accomodates for very long lines).
      */
-    const MAX_READ_BYTES_PER_LINE = 32768;
+    const MAX_READ_BYTES_PER_LINE = 0;
 
     /** @var resource Pointer to the CSV file to read */
     protected $filePointer;

src/Spout/Reader/ODS/SheetIterator.php

@@ -116,7 +116,7 @@ class SheetIterator implements IteratorInterface
         $escapedSheetName = $this->xmlReader->getAttribute(self::XML_ATTRIBUTE_TABLE_NAME);
         $sheetName = $this->escaper->unescape($escapedSheetName);
 
-        return new Sheet($this->xmlReader, $sheetName, $this->currentSheetIndex, $this->options);
+        return new Sheet($this->xmlReader, $this->currentSheetIndex, $sheetName, $this->options);
     }
 
     /**

src/Spout/Reader/XLSX/Helper/CellHelper.php

@@ -31,6 +31,9 @@ class CellHelper
      */
     public static function fillMissingArrayIndexes($dataArray, $fillValue = '')
     {
+        if (empty($dataArray)) {
+            return [];
+        }
         $existingIndexes = array_keys($dataArray);
 
         $newIndexes = array_fill_keys(range(0, max($existingIndexes)), $fillValue);

src/Spout/Reader/XLSX/Helper/SharedStringsHelper.php

@@ -234,11 +234,6 @@ class SharedStringsHelper
         $textValue = '';
 
         foreach ($textNodes as $nodeIndex => $textNode) {
-            if ($nodeIndex !== 0) {
-                // add a space between each "t" node
-                $textValue .= ' ';
-            }
-
             $textNodeAsString = $textNode->__toString();
             $shouldPreserveWhitespace = $this->shouldPreserveWhitespace($textNode);
 

src/Spout/Reader/XLSX/RowIterator.php

@@ -220,9 +220,8 @@ class RowIterator implements IteratorInterface
     {
         // Read dimensions of the sheet
         $dimensionRef = $xmlReader->getAttribute(self::XML_ATTRIBUTE_REF); // returns 'A1:M13' for instance (or 'A1' for empty sheet)
-        if (preg_match('/[A-Z\d]+:([A-Z\d]+)/', $dimensionRef, $matches)) {
-            $lastCellIndex = $matches[1];
-            $this->numColumns = CellHelper::getColumnIndexFromCellIndex($lastCellIndex) + 1;
+        if (preg_match('/[A-Z]+\d+:([A-Z]+\d+)/', $dimensionRef, $matches)) {
+            $this->numColumns = CellHelper::getColumnIndexFromCellIndex($matches[1]) + 1;
         }
 
         return XMLProcessor::PROCESSING_CONTINUE;

src/Spout/Writer/AbstractWriter.php

@@ -350,6 +350,10 @@ abstract class AbstractWriter implements WriterInterface
      */
     public function close()
     {
+        if (!$this->isWriterOpened) {
+            return;
+        }
+
         $this->closeWriter();
 
         if (is_resource($this->filePointer)) {
@@ -378,4 +382,3 @@ abstract class AbstractWriter implements WriterInterface
         }
     }
 }
-

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));
     }
@@ -75,43 +84,58 @@ class Sheet
      */
     public function setName($name)
     {
-        if (!$this->isNameValid($name)) {
-            $errorMessage = "The sheet's name is invalid. It did not meet at least one of these requirements:\n";
-            $errorMessage .= " - It should not be blank\n";
-            $errorMessage .= " - It should not exceed 31 characters\n";
-            $errorMessage .= " - It should not contain these characters: \\ / ? * : [ or ]\n";
-            $errorMessage .= " - It should be unique";
-            throw new InvalidSheetNameException($errorMessage);
-        }
+        $this->throwIfNameIsInvalid($name);
 
         $this->name = $name;
-        self::$SHEETS_NAME_USED[$this->index] = $name;
+        self::$SHEETS_NAME_USED[$this->associatedWorkbookId][$this->index] = $name;
 
         return $this;
     }
 
     /**
-     * Returns whether the given sheet's name is valid.
+     * Throws an exception if the given sheet's name is not valid.
      * @see Sheet::setName for validity rules.
      *
      * @param string $name
-     * @return bool TRUE if the name is valid, FALSE otherwise.
+     * @return void
+     * @throws \Box\Spout\Writer\Exception\InvalidSheetNameException If the sheet's name is invalid.
      */
-    protected function isNameValid($name)
+    protected function throwIfNameIsInvalid($name)
     {
         if (!is_string($name)) {
-            return false;
+            $actualType = gettype($name);
+            $errorMessage = "The sheet's name is invalid. It must be a string ($actualType given).";
+            throw new InvalidSheetNameException($errorMessage);
         }
 
+        $failedRequirements = [];
         $nameLength = $this->stringHelper->getStringLength($name);
 
-        return (
-            $nameLength > 0 &&
-            $nameLength <= self::MAX_LENGTH_SHEET_NAME &&
-            !$this->doesContainInvalidCharacters($name) &&
-            $this->isNameUnique($name) &&
-            !$this->doesStartOrEndWithSingleQuote($name)
-        );
+        if (!$this->isNameUnique($name)) {
+            $failedRequirements[] = 'It should be unique';
+        } else {
+            if ($nameLength === 0) {
+                $failedRequirements[] = 'It should not be blank';
+            } else {
+                if ($nameLength > self::MAX_LENGTH_SHEET_NAME) {
+                    $failedRequirements[] = 'It should not exceed 31 characters';
+                }
+
+                if ($this->doesContainInvalidCharacters($name)) {
+                    $failedRequirements[] = 'It should not contain these characters: \\ / ? * : [ or ]';
+                }
+
+                if ($this->doesStartOrEndWithSingleQuote($name)) {
+                    $failedRequirements[] = 'It should not start or end with a single quote';
+                }
+            }
+        }
+
+        if (count($failedRequirements) !== 0) {
+            $errorMessage = "The sheet's name (\"$name\") is invalid. It did not respect these rules:\n - ";
+            $errorMessage .= implode("\n - ", $failedRequirements);
+            throw new InvalidSheetNameException($errorMessage);
+        }
     }
 
     /**
@@ -148,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);

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

@@ -4,6 +4,7 @@ namespace Box\Spout\Writer\XLSX\Internal;
 
 use Box\Spout\Common\Exception\InvalidArgumentException;
 use Box\Spout\Common\Exception\IOException;
+use Box\Spout\Common\Helper\StringHelper;
 use Box\Spout\Writer\Common\Helper\CellHelper;
 use Box\Spout\Writer\Common\Internal\WorksheetInterface;
 
@@ -16,6 +17,14 @@ use Box\Spout\Writer\Common\Internal\WorksheetInterface;
  */
 class Worksheet implements WorksheetInterface
 {
+    /**
+     * Maximum number of characters a cell can contain
+     * @see https://support.office.com/en-us/article/Excel-specifications-and-limits-16c69c74-3d6a-4aaf-ba35-e6eb276e8eaa [Excel 2007]
+     * @see https://support.office.com/en-us/article/Excel-specifications-and-limits-1672b34d-7043-467e-8e27-269d656771c3 [Excel 2010]
+     * @see https://support.office.com/en-us/article/Excel-specifications-and-limits-ca36e2dc-1f09-4620-b726-67c00b05040f [Excel 2013/2016]
+     */
+    const MAX_CHARACTERS_PER_CELL = 32767;
+
     const SHEET_XML_FILE_HEADER = <<<EOD
 <?xml version="1.0" encoding="UTF-8" standalone="yes"?>
 <worksheet xmlns="http://schemas.openxmlformats.org/spreadsheetml/2006/main" xmlns:r="http://schemas.openxmlformats.org/officeDocument/2006/relationships">
@@ -39,6 +48,9 @@ EOD;
     /** @var \Box\Spout\Common\Escaper\XLSX Strings escaper */
     protected $stringsEscaper;
 
+    /** @var \Box\Spout\Common\Helper\StringHelper String helper */
+    protected $stringHelper;
+
     /** @var Resource Pointer to the sheet data file (e.g. xl/worksheets/sheet1.xml) */
     protected $sheetFilePointer;
 
@@ -62,6 +74,7 @@ EOD;
 
         /** @noinspection PhpUnnecessaryFullyQualifiedNameInspection */
         $this->stringsEscaper = \Box\Spout\Common\Escaper\XLSX::getInstance();
+        $this->stringHelper = new StringHelper();
 
         $this->worksheetFilePath = $worksheetFilesFolder . '/' . strtolower($this->externalSheet->getName()) . '.xml';
         $this->startSheet();
@@ -192,7 +205,7 @@ EOD;
      * @param mixed $cellValue
      * @param int $styleId
      * @return string
-     * @throws InvalidArgumentException
+     * @throws InvalidArgumentException If the given value cannot be processed
      */
     private function getCellXML($rowIndex, $cellNumber, $cellValue, $styleId)
     {
@@ -201,12 +214,7 @@ EOD;
         $cellXML .= ' s="' . $styleId . '"';
 
         if (CellHelper::isNonEmptyString($cellValue)) {
-            if ($this->shouldUseInlineStrings) {
-                $cellXML .= ' t="inlineStr"><is><t>' . $this->stringsEscaper->escape($cellValue) . '</t></is></c>';
-            } else {
-                $sharedStringId = $this->sharedStringsHelper->writeString($cellValue);
-                $cellXML .= ' t="s"><v>' . $sharedStringId . '</v></c>';
-            }
+            $cellXML .= $this->getCellXMLFragmentForNonEmptyString($cellValue);
         } else if (CellHelper::isBoolean($cellValue)) {
             $cellXML .= ' t="b"><v>' . intval($cellValue) . '</v></c>';
         } else if (CellHelper::isNumeric($cellValue)) {
@@ -226,6 +234,29 @@ EOD;
         return $cellXML;
     }
 
+    /**
+     * Returns the XML fragment for a cell containing a non empty string
+     *
+     * @param string $cellValue The cell value
+     * @return string The XML fragment representing the cell
+     * @throws InvalidArgumentException If the string exceeds the maximum number of characters allowed per cell
+     */
+    private function getCellXMLFragmentForNonEmptyString($cellValue)
+    {
+        if ($this->stringHelper->getStringLength($cellValue) > self::MAX_CHARACTERS_PER_CELL) {
+            throw new InvalidArgumentException('Trying to add a value that exceeds the maximum number of characters allowed in a cell (32,767)');
+        }
+
+        if ($this->shouldUseInlineStrings) {
+            $cellXMLFragment = ' t="inlineStr"><is><t>' . $this->stringsEscaper->escape($cellValue) . '</t></is></c>';
+        } else {
+            $sharedStringId = $this->sharedStringsHelper->writeString($cellValue);
+            $cellXMLFragment = ' t="s"><v>' . $sharedStringId . '</v></c>';
+        }
+
+        return $cellXMLFragment;
+    }
+
     /**
      * Closes the worksheet
      *

tests/Spout/Reader/ODS/SheetTest.php

@@ -0,0 +1,51 @@
+<?php
+
+namespace Box\Spout\Reader\ODS;
+
+use Box\Spout\Common\Type;
+use Box\Spout\Reader\ReaderFactory;
+use Box\Spout\TestUsingResource;
+
+/**
+ * Class SheetTest
+ *
+ * @package Box\Spout\Reader\ODS
+ */
+class SheetTest extends \PHPUnit_Framework_TestCase
+{
+    use TestUsingResource;
+
+    /**
+     * @return void
+     */
+    public function testNextSheetShouldReturnCorrectSheetInfos()
+    {
+        $sheets = $this->openFileAndReturnSheets('two_sheets_with_custom_names.ods');
+
+        $this->assertEquals('Sheet First', $sheets[0]->getName());
+        $this->assertEquals(0, $sheets[0]->getIndex());
+
+        $this->assertEquals('Sheet Last', $sheets[1]->getName());
+        $this->assertEquals(1, $sheets[1]->getIndex());
+    }
+
+    /**
+     * @param string $fileName
+     * @return Sheet[]
+     */
+    private function openFileAndReturnSheets($fileName)
+    {
+        $resourcePath = $this->getResourcePath($fileName);
+        $reader = ReaderFactory::create(Type::ODS);
+        $reader->open($resourcePath);
+
+        $sheets = [];
+        foreach ($reader->getSheetIterator() as $sheet) {
+            $sheets[] = $sheet;
+        }
+
+        $reader->close();
+
+        return $sheets;
+    }
+}

tests/Spout/Reader/XLSX/Helper/CellHelperTest.php

@@ -10,14 +10,25 @@ namespace Box\Spout\Reader\XLSX\Helper;
 class CellHelperTest extends \PHPUnit_Framework_TestCase
 {
     /**
-     * @return void
+     * @return array
      */
-    public function testFillMissingArrayIndexes()
+    public function dataProviderForTestFillMissingArrayIndexes()
     {
-        $arrayToFill = [1 => 1, 3 => 3];
-        $filledArray = CellHelper::fillMissingArrayIndexes($arrayToFill, 'FILL');
+        return [
+            [ null, [] ],
+            [ [], [] ],
+            [ [1 => 1, 3 => 3], ['FILL', 1, 'FILL', 3] ]
+        ];
+    }
 
-        $expectedFilledArray = ['FILL', 1, 'FILL', 3];
+    /**
+     * @dataProvider dataProviderForTestFillMissingArrayIndexes
+     * @param array $arrayToFill
+     * @param array $expectedFilledArray
+     */
+    public function testFillMissingArrayIndexes($arrayToFill, array $expectedFilledArray)
+    {
+        $filledArray = CellHelper::fillMissingArrayIndexes($arrayToFill, 'FILL');
         $this->assertEquals($expectedFilledArray, $filledArray);
     }
 

tests/Spout/Writer/CSV/WriterTest.php

@@ -61,6 +61,23 @@ class WriterTest extends \PHPUnit_Framework_TestCase
         $writer->close();
     }
 
+    /**
+     * @return void
+     */
+    public function testCloseShouldNoopWhenWriterIsNotOpened()
+    {
+        $fileName = 'test_double_close_calls.csv';
+        $this->createGeneratedFolderIfNeeded($fileName);
+        $resourcePath = $this->getGeneratedResourcePath($fileName);
+
+        $writer = WriterFactory::create(Type::CSV);
+        $writer->close(); // This call should not cause any error
+
+        $writer->openToFile($fileName);
+        $writer->close();
+        $writer->close(); // This call should not cause any error
+    }
+
     /**
      * @return void
      */

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);
     }
 }

tests/Spout/Writer/ODS/WriterTest.php

@@ -67,7 +67,7 @@ class WriterTest extends \PHPUnit_Framework_TestCase
     /**
      * @expectedException \Box\Spout\Writer\Exception\WriterAlreadyOpenedException
      */
-    public function testsetShouldCreateNewSheetsAutomaticallyShouldThrowExceptionIfCalledAfterOpeningWriter()
+    public function testSetShouldCreateNewSheetsAutomaticallyShouldThrowExceptionIfCalledAfterOpeningWriter()
     {
         $fileName = 'file_that_wont_be_written.ods';
         $filePath = $this->getGeneratedResourcePath($fileName);
@@ -168,6 +168,23 @@ class WriterTest extends \PHPUnit_Framework_TestCase
         $this->assertEquals($firstSheet, $writer->getCurrentSheet(), 'The current sheet should be the first one.');
     }
 
+    /**
+     * @return void
+     */
+    public function testCloseShouldNoopWhenWriterIsNotOpened()
+    {
+        $fileName = 'test_double_close_calls.ods';
+        $this->createGeneratedFolderIfNeeded($fileName);
+        $resourcePath = $this->getGeneratedResourcePath($fileName);
+
+        $writer = WriterFactory::create(Type::ODS);
+        $writer->close(); // This call should not cause any error
+
+        $writer->openToFile($fileName);
+        $writer->close();
+        $writer->close(); // This call should not cause any error
+    }
+
     /**
      * @return void
      */

tests/Spout/Writer/XLSX/WriterTest.php

@@ -6,6 +6,7 @@ use Box\Spout\Common\Exception\SpoutException;
 use Box\Spout\Common\Type;
 use Box\Spout\TestUsingResource;
 use Box\Spout\Writer\WriterFactory;
+use Box\Spout\Writer\XLSX\Internal\Worksheet;
 
 /**
  * Class WriterTest
@@ -98,6 +99,19 @@ class WriterTest extends \PHPUnit_Framework_TestCase
     public function testAddRowShouldThrowExceptionIfUnsupportedDataTypePassedIn()
     {
         $fileName = 'test_add_row_should_throw_exception_if_unsupported_data_type_passed_in.xlsx';
+        $dataRows = [
+            [str_repeat('a', Worksheet::MAX_CHARACTERS_PER_CELL + 1)],
+        ];
+
+        $this->writeToXLSXFile($dataRows, $fileName);
+    }
+
+    /**
+     * @expectedException \Box\Spout\Common\Exception\InvalidArgumentException
+     */
+    public function testAddRowShouldThrowExceptionIfWritingStringExceedingMaxNumberOfCharactersAllowedPerCell()
+    {
+        $fileName = 'test_add_row_should_throw_exception_if_string_exceeds_max_num_chars_allowed_per_cell.xlsx';
         $dataRows = [
             [new \stdClass()],
         ];
@@ -180,6 +194,23 @@ class WriterTest extends \PHPUnit_Framework_TestCase
         $this->assertEquals($firstSheet, $writer->getCurrentSheet(), 'The current sheet should be the first one.');
     }
 
+    /**
+     * @return void
+     */
+    public function testCloseShouldNoopWhenWriterIsNotOpened()
+    {
+        $fileName = 'test_double_close_calls.xlsx';
+        $this->createGeneratedFolderIfNeeded($fileName);
+        $resourcePath = $this->getGeneratedResourcePath($fileName);
+
+        $writer = WriterFactory::create(Type::XLSX);
+        $writer->close(); // This call should not cause any error
+
+        $writer->openToFile($fileName);
+        $writer->close();
+        $writer->close(); // This call should not cause any error
+    }
+
     /**
      * @return void
      */