From 7efab5576da2a813a32683b573f770a5e8983eea Mon Sep 17 00:00:00 2001 From: Adrien Loison Date: Fri, 21 Aug 2015 15:21:35 -0700 Subject: [PATCH] Detection of invalid sheet name Based on Excel requirements: - it should not be blank - it should not exceed 31 characters - it should not contain these characters: \ / ? * [ or ] - it should be unique --- README.md | 9 ++ .../Exception/InvalidSheetNameException.php | 12 ++ src/Spout/Writer/XLSX/Sheet.php | 119 +++++++++++++++++- tests/Spout/Writer/XLSX/SheetTest.php | 103 +++++++++++++-- 4 files changed, 234 insertions(+), 9 deletions(-) create mode 100644 src/Spout/Writer/Exception/InvalidSheetNameException.php diff --git a/README.md b/README.md index 45197ad..22e16dd 100644 --- a/README.md +++ b/README.md @@ -263,6 +263,15 @@ $sheet = $writer->getCurrentSheet(); $sheetName = $sheet->setName('My custom name'); ``` +> Please note that Excel has some restrictions on the sheet's name: +> * it should not exceed 31 characters +> * it should not contain these characters: \ / ? * [ or ] +> * it should not be blank +> * it should be unique +> +> Handling these restrictions is the developer's responsibility. Spout does not try to automatically change the sheet's name, as one may rely on this name to be exactly what was passed in. + + ### Fluent interface Because fluent interfaces are great, you can use them with Spout: diff --git a/src/Spout/Writer/Exception/InvalidSheetNameException.php b/src/Spout/Writer/Exception/InvalidSheetNameException.php new file mode 100644 index 0000000..501b97b --- /dev/null +++ b/src/Spout/Writer/Exception/InvalidSheetNameException.php @@ -0,0 +1,12 @@ + [SHEET_NAME] keeping track of sheets' name to enforce uniqueness */ + protected static $SHEETS_NAME_USED = []; + /** @var int Index of the sheet, based on order of creation (zero-based) */ protected $index; @@ -24,7 +35,7 @@ class Sheet public function __construct($sheetIndex) { $this->index = $sheetIndex; - $this->name = self::DEFAULT_SHEET_NAME_PREFIX . ($sheetIndex + 1); + $this->setName(self::DEFAULT_SHEET_NAME_PREFIX . ($sheetIndex + 1)); } /** @@ -44,12 +55,116 @@ class Sheet } /** + * Sets the name of the sheet. Note that Excel has some restrictions on the name: + * - it should not be blank + * - it should not exceed 31 characters + * - it should not contain these characters: \ / ? * [ or ] + * - it should be unique + * * @param string $name Name of the sheet * @return Sheet + * @throws \Box\Spout\Writer\Exception\InvalidSheetNameException If the sheet's name is invalid. */ 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->name = $name; + self::$SHEETS_NAME_USED[$this->index] = $name; + return $this; } + + /** + * Returns whether the given sheet's name is valid. + * @see Sheet::setName for validity rules. + * + * @param string $name + * @return bool TRUE if the name is valid, FALSE otherwise. + */ + protected function isNameValid($name) + { + if (!is_string($name)) { + return false; + } + + $nameLength = $this->getStringLength($name); + $hasValidLength = ($nameLength > 0 && $nameLength <= self::MAX_LENGTH_SHEET_NAME); + $containsInvalidCharacters = $this->doesContainInvalidCharacters($name); + $isNameUnique = $this->isNameUnique($name); + + return ($hasValidLength && !$containsInvalidCharacters && $isNameUnique); + } + + /** + * Returns the length of the given string. + * It uses the multi-bytes function is available. + * @see strlen + * @see mb_strlen + * + * @param string $string + * @return int + */ + protected function getStringLength($string) + { + return extension_loaded('mbstring') ? mb_strlen($string) : strlen($string); + } + + /** + * Returns the position of the given character/substring in the given string. + * It uses the multi-bytes function is available. + * @see strpos + * @see mb_strpos + * + * @param string $string Haystack + * @param string $char Needle + * @return int Index of the char in the string if found (started at 0) or -1 if not found + */ + protected function getCharPosition($string, $char) + { + $position = extension_loaded('mbstring') ? mb_strpos($string, $char) : strpos($string, $char); + return ($position !== false) ? $position : -1; + } + + /** + * Returns whether the given name contains at least one invalid character. + * @see Sheet::$INVALID_CHARACTERS_IN_SHEET_NAME for the full list. + * + * @param string $name + * @return bool TRUE if the name contains invalid characters, FALSE otherwise. + */ + protected function doesContainInvalidCharacters($name) + { + foreach (self::$INVALID_CHARACTERS_IN_SHEET_NAME as $invalidCharacter) { + if ($this->getCharPosition($name, $invalidCharacter) !== -1) { + return true; + } + } + + return false; + } + + /** + * Returns whether the given name is unique. + * + * @param string $name + * @return bool TRUE if the name is unique, FALSE otherwise. + */ + protected function isNameUnique($name) + { + foreach (self::$SHEETS_NAME_USED as $sheetIndex => $sheetName) { + if ($sheetIndex !== $this->index && $sheetName === $name) { + return false; + } + } + + return true; + } } diff --git a/tests/Spout/Writer/XLSX/SheetTest.php b/tests/Spout/Writer/XLSX/SheetTest.php index d9d6020..76629c2 100644 --- a/tests/Spout/Writer/XLSX/SheetTest.php +++ b/tests/Spout/Writer/XLSX/SheetTest.php @@ -20,7 +20,7 @@ class SheetTest extends \PHPUnit_Framework_TestCase */ public function testGetSheetIndex() { - $sheets = $this->writeDataAndReturnSheets('test_get_sheet_index.xlsx'); + $sheets = $this->writeDataToMulitpleSheetsAndReturnSheets('test_get_sheet_index.xlsx'); $this->assertEquals(2, count($sheets), '2 sheets should have been created'); $this->assertEquals(0, $sheets[0]->getIndex(), 'The first sheet should be index 0'); @@ -32,7 +32,7 @@ class SheetTest extends \PHPUnit_Framework_TestCase */ public function testGetSheetName() { - $sheets = $this->writeDataAndReturnSheets('test_get_sheet_name.xlsx'); + $sheets = $this->writeDataToMulitpleSheetsAndReturnSheets('test_get_sheet_name.xlsx'); $this->assertEquals(2, count($sheets), '2 sheets should have been created'); $this->assertEquals('Sheet1', $sheets[0]->getName(), 'Invalid name for the first sheet'); @@ -45,27 +45,115 @@ class SheetTest extends \PHPUnit_Framework_TestCase public function testSetSheetNameShouldCreateSheetWithCustomName() { $fileName = 'test_set_name_should_create_sheet_with_custom_name.xlsx'; + $customSheetName = 'CustomName'; + $this->writeDataAndReturnSheetWithCustomName($fileName, $customSheetName); + + $this->assertSheetNameEquals($customSheetName, $fileName, "The sheet name should have been changed to '$customSheetName'"); + } + + /** + * @return array + */ + public function dataProviderForInvalidSheetNames() + { + return [ + [null], + [21], + [''], + ['this title exceeds the 31 characters limit'], + ['Illegal \\'], + ['Illegal /'], + ['Illegal ?'], + ['Illegal *'], + ['Illegal ['], + ['Illegal ]'], + ]; + } + + /** + * @dataProvider dataProviderForInvalidSheetNames + * @expectedException \Box\Spout\Writer\Exception\InvalidSheetNameException + * + * @param string $customSheetName + * @return void + */ + public function testSetSheetNameShouldThrowOnInvalidName($customSheetName) + { + $fileName = 'test_set_name_with_invalid_name_should_throw_exception.xlsx'; + $this->writeDataAndReturnSheetWithCustomName($fileName, $customSheetName); + } + + /** + * @return void + */ + public function testSetSheetNameShouldNotThrowWhenSettingSameNameAsCurrentOne() + { + $fileName = 'test_set_name_with_same_as_current.xlsx'; $this->createGeneratedFolderIfNeeded($fileName); $resourcePath = $this->getGeneratedResourcePath($fileName); $writer = WriterFactory::create(Type::XLSX); $writer->openToFile($resourcePath); - $customSheetName = 'CustomName'; + $customSheetName = 'Sheet name'; $sheet = $writer->getCurrentSheet(); $sheet->setName($customSheetName); + $sheet->setName($customSheetName); $writer->addRow(['xlsx--11', 'xlsx--12']); $writer->close(); - $this->assertSheetNameEquals($customSheetName, $resourcePath, "The sheet name should have been changed to '$customSheetName'"); + $this->assertSheetNameEquals($customSheetName, $fileName, "The sheet name should have been changed to '$customSheetName'"); + } + + /** + * @expectedException \Box\Spout\Writer\Exception\InvalidSheetNameException + * @return void + */ + public function testSetSheetNameShouldThrowWhenNameIsAlreadyUsed() + { + $fileName = 'test_set_name_with_non_unique_name.xlsx'; + $this->createGeneratedFolderIfNeeded($fileName); + $resourcePath = $this->getGeneratedResourcePath($fileName); + + $writer = WriterFactory::create(Type::XLSX); + $writer->openToFile($resourcePath); + + $customSheetName = 'Sheet name'; + + $sheet = $writer->getCurrentSheet(); + $sheet->setName($customSheetName); + + $writer->addNewSheetAndMakeItCurrent(); + $sheet = $writer->getCurrentSheet(); + $sheet->setName($customSheetName); + } + + /** + * @param string $fileName + * @param string $sheetName + * @return Sheet + */ + private function writeDataAndReturnSheetWithCustomName($fileName, $sheetName) + { + $this->createGeneratedFolderIfNeeded($fileName); + $resourcePath = $this->getGeneratedResourcePath($fileName); + + $writer = WriterFactory::create(Type::XLSX); + $writer->openToFile($resourcePath); + + $sheet = $writer->getCurrentSheet(); + $sheet->setName($sheetName); + + $writer->addRow(['xlsx--11', 'xlsx--12']); + $writer->close(); } /** * @param string $fileName * @return Sheet[] */ - private function writeDataAndReturnSheets($fileName) + private function writeDataToMulitpleSheetsAndReturnSheets($fileName) { $this->createGeneratedFolderIfNeeded($fileName); $resourcePath = $this->getGeneratedResourcePath($fileName); @@ -85,12 +173,13 @@ class SheetTest extends \PHPUnit_Framework_TestCase /** * @param string $expectedName - * @param string $resourcePath + * @param string $fileName * @param string $message * @return void */ - private function assertSheetNameEquals($expectedName, $resourcePath, $message = '') + private function assertSheetNameEquals($expectedName, $fileName, $message = '') { + $resourcePath = $this->getGeneratedResourcePath($fileName); $pathToWorkbookFile = $resourcePath . '#xl/workbook.xml'; $xmlContents = file_get_contents('zip://' . $pathToWorkbookFile);