Merge pull request #60 from Lewiscowles1986/Update-XLSX-Reader

Separated getCellValue into multiple functions

.travis.yml

@@ -8,7 +8,11 @@ php:
   - hhvm
 
 install:
-  - composer self-update
+  - composer install --no-interaction
-  - composer install --prefer-source
 
-script: phpunit
+script:
+  - mkdir -p build/logs
+  - php vendor/bin/phpunit --coverage-clover build/logs/clover.xml
+
+after_script:
+  - php vendor/bin/coveralls -v

README.md

@@ -3,7 +3,9 @@
 [![Latest Stable Version](https://poser.pugx.org/box/spout/v/stable)](https://packagist.org/packages/box/spout)
 [![Project Status](http://opensource.box.com/badges/active.svg)](http://opensource.box.com/badges)
 [![Build Status](https://travis-ci.org/box/spout.png?branch=master)](http://travis-ci.org/box/spout)
-[![Total Downloads](https://poser.pugx.org/box/spout/downloads)](https://packagist.org/packages/box/spout) [![License](https://poser.pugx.org/box/spout/license)](https://packagist.org/packages/box/spout)
+[![Coverage Status](https://coveralls.io/repos/box/spout/badge.svg?branch=master)](https://coveralls.io/r/box/spout?branch=master)
+[![Total Downloads](https://poser.pugx.org/box/spout/downloads)](https://packagist.org/packages/box/spout)
+[![License](https://poser.pugx.org/box/spout/license)](https://packagist.org/packages/box/spout)
 
 Spout is a PHP library to read and write CSV and XLSX files, in a fast and scalable way.
 Contrary to other file readers or writers, it is capable of processing very large files while keeping the memory usage really low (less than 10MB).

composer.json

@@ -18,7 +18,8 @@
         "ext-simplexml": "*"
     },
     "require-dev": {
-        "phpunit/phpunit": ">=3.7"
+        "phpunit/phpunit": ">=3.7",
+        "satooshi/php-coveralls": "^0.6.1"
     },
     "autoload": {
         "psr-4": {

phpunit.xml

@@ -14,10 +14,6 @@
         </testsuite>
     </testsuites>
 
-    <logging>
-        <log type="coverage-html" target="tests/coverage/"/>
-    </logging>
-
     <filter>
         <whitelist>
             <directory suffix=".php">src/</directory>

src/Spout/Common/Helper/FileSystemHelper.php

@@ -36,7 +36,7 @@ class FileSystemHelper
     {
         $this->throwIfOperationNotInBaseFolder($parentFolderPath);
 
-        $folderPath = $parentFolderPath . DIRECTORY_SEPARATOR . $folderName;
+        $folderPath = $parentFolderPath . '/' . $folderName;
 
         $wasCreationSuccessful = mkdir($folderPath, 0777, true);
         if (!$wasCreationSuccessful) {
@@ -60,7 +60,7 @@ class FileSystemHelper
     {
         $this->throwIfOperationNotInBaseFolder($parentFolderPath);
 
-        $filePath = $parentFolderPath . DIRECTORY_SEPARATOR . $fileName;
+        $filePath = $parentFolderPath . '/' . $fileName;
 
         $wasCreationSuccessful = file_put_contents($filePath, $fileContents);
         if (!$wasCreationSuccessful) {

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

@@ -112,7 +112,7 @@ class SharedStringsHelper
         $escaper = new \Box\Spout\Common\Escaper\XLSX();
 
         $sharedStringsFilePath = $this->getSharedStringsFilePath();
-        if ($xmlReader->open($sharedStringsFilePath, null, LIBXML_NONET) === false) {
+        if ($xmlReader->open($sharedStringsFilePath, null, LIBXML_NOENT|LIBXML_NONET) === false) {
             throw new IOException('Could not open "' . self::SHARED_STRINGS_XML_FILE_PATH . '".');
         }
 
@@ -162,12 +162,11 @@ class SharedStringsHelper
     }
 
     /**
-     * @return string The path to the shared strings XML file, working cross-platforms
+     * @return string The path to the shared strings XML file
      */
     protected function getSharedStringsFilePath()
     {
-        $sharedStringsXmlFilePath = str_replace('/', DIRECTORY_SEPARATOR, self::SHARED_STRINGS_XML_FILE_PATH);
+        return 'zip://' . $this->filePath . '#' . self::SHARED_STRINGS_XML_FILE_PATH;
-        return 'zip://' . $this->filePath . '#' . $sharedStringsXmlFilePath;
     }
 
     /**
@@ -250,7 +249,7 @@ class SharedStringsHelper
     protected function getSharedStringTempFilePath($sharedStringIndex)
     {
         $numTempFile = intval($sharedStringIndex / self::MAX_NUM_STRINGS_PER_TEMP_FILE);
-        return $this->tempFolder . DIRECTORY_SEPARATOR . 'sharedstrings' . $numTempFile;
+        return $this->tempFolder . '/sharedstrings' . $numTempFile;
     }
 
     /**

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

@@ -186,8 +186,7 @@ class WorksheetHelper
      */
     protected function getFileAsXMLElementWithNamespace($xmlFilePath, $mainNamespace)
     {
-        $normalizedXmlFilePath = str_replace('/', DIRECTORY_SEPARATOR, $xmlFilePath);
+        $xmlContents = $this->globalFunctionsHelper->file_get_contents('zip://' . $this->filePath . '#' . $xmlFilePath);
-        $xmlContents = $this->globalFunctionsHelper->file_get_contents('zip://' . $this->filePath . '#' . $normalizedXmlFilePath);
 
         $xmlElement = new \SimpleXMLElement($xmlContents);
         $xmlElement->registerXPathNamespace('ns', $mainNamespace);

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

@@ -33,12 +33,11 @@ class Worksheet
 
     /**
      * @return string Path of the XML file containing the worksheet data,
-     *                without the leading slash and working cross-platforms.
+     *                without the leading slash.
      */
     public function getDataXmlFilePath()
     {
-        $dataXmlFilePath = ltrim($this->dataXmlFilePath, '/');
+        return ltrim($this->dataXmlFilePath, '/');
-        return str_replace('/', DIRECTORY_SEPARATOR, $dataXmlFilePath);
     }
 
     /**

src/Spout/Reader/XLSX.php

@@ -19,6 +19,14 @@ use Box\Spout\Reader\Helper\XLSX\WorksheetHelper;
  */
 class XLSX extends AbstractReader
 {
+    const CELL_TYPE_INLINE_STRING = 'inlineStr';
+    const CELL_TYPE_STR = 'str';
+    const CELL_TYPE_SHARED_STRING = 's';
+    const CELL_TYPE_BOOLEAN = 'b';
+    const CELL_TYPE_NUMERIC = 'n';
+    const CELL_TYPE_DATE = 'd';
+    const CELL_TYPE_ERROR = 'e';
+    
     /** @var string Real path of the file to read */
     protected $filePath;
 
@@ -158,7 +166,7 @@ class XLSX extends AbstractReader
         $worksheetDataXMLFilePath = $worksheet->getDataXmlFilePath();
 
         $worksheetDataFilePath = 'zip://' . $this->filePath . '#' . $worksheetDataXMLFilePath;
-        if ($this->xmlReader->open($worksheetDataFilePath, null, LIBXML_NONET) === false) {
+        if ($this->xmlReader->open($worksheetDataFilePath, null, LIBXML_NOENT|LIBXML_NONET) === false) {
             throw new IOException('Could not open "' . $worksheetDataXMLFilePath . '".');
         }
     }
@@ -224,7 +232,115 @@ class XLSX extends AbstractReader
     }
 
     /**
-     * Returns the (unescaped) cell value associated to the given XML node.
+     * Returns the cell's string value from a node's nested value node
+     *
+     * @param \DOMNode $node
+     * @return string The value associated with the cell
+     */
+    protected function getVNodeValue($node)
+    {
+        // for cell types having a "v" tag containing the value.
+        // if not, the returned value should be empty string.
+        $vNode = $node->getElementsByTagName('v')->item(0);
+        if ($vNode !== null) {
+            return $vNode->nodeValue;
+        }
+        return "";
+    }
+
+    /**
+     * Returns the cell String value where string is inline.
+     *
+     * @param \DOMNode $node
+     * @param \Box\Spout\Common\Escaper\XLSX $escaper
+     * @return string The value associated with the cell (null when the cell has an error)
+     */
+    protected function formatInlineStringCellValue($node, $escaper)
+    {
+        // inline strings are formatted this way:
+        // <c r="A1" t="inlineStr"><is><t>[INLINE_STRING]</t></is></c>
+        $tNode = $node->getElementsByTagName('t')->item(0);
+        $escapedCellValue = trim($tNode->nodeValue);
+        $cellValue = $escaper->unescape($escapedCellValue);
+        return $cellValue;
+    }
+
+    /**
+     * Returns the cell String value from shared-strings file using nodeValue index.
+     *
+     * @param string $nodeValue
+     * @param \Box\Spout\Common\Escaper\XLSX $escaper
+     * @return string The value associated with the cell (null when the cell has an error)
+     */
+    protected function formatSharedStringCellValue($nodeValue, $escaper)
+    {
+        // shared strings are formatted this way:
+        // <c r="A1" t="s"><v>[SHARED_STRING_INDEX]</v></c>
+        $sharedStringIndex = intval($nodeValue);
+        $escapedCellValue = $this->sharedStringsHelper->getStringAtIndex($sharedStringIndex);
+        $cellValue = $escaper->unescape($escapedCellValue);
+        return $cellValue;
+    }
+
+    /**
+     * Returns the cell String value, where string is stored in value node.
+     *
+     * @param string $nodeValue
+     * @param \Box\Spout\Common\Escaper\XLSX $escaper
+     * @return string The value associated with the cell (null when the cell has an error)
+     */
+    protected function formatStrCellValue($nodeValue, $escaper)
+    {
+        $escapedCellValue = trim($nodeValue);
+        $cellValue = $escaper->unescape($escapedCellValue);
+        return $cellValue;
+    }
+
+    /**
+     * Returns the cell Numeric value from string of nodeValue.
+     *
+     * @param string $nodeValue
+     * @param \Box\Spout\Common\Escaper\XLSX $escaper
+     * @return int|float The value associated with the cell
+     */
+    protected function formatNumericCellValue($nodeValue)
+    {
+        $cellValue = is_int($nodeValue) ? intval($nodeValue) : floatval($nodeValue);
+        return $cellValue;
+    }
+
+    /**
+     * Returns the cell Boolean value from a specific node's Value.
+     *
+     * @param string $nodeValue
+     * @return bool The value associated with the cell
+     */
+    protected function formatBooleanCellValue($nodeValue)
+    {
+        // !! is similar to boolval()
+        $cellValue = !!$nodeValue;
+        return $cellValue;
+    }
+
+    /**
+     * Returns a cell's PHP Date value, associated to the given stored nodeValue.
+     *
+     * @param string $nodeValue
+     * @param \Box\Spout\Common\Escaper\XLSX $escaper
+     * @return DateTime|null The value associated with the cell (null when the cell has an error)
+     */
+    protected function formatDateCellValue($nodeValue)
+    {
+        try { // Mitigate thrown Exception on invalid date-time format (http://php.net/manual/en/datetime.construct.php)
+            $cellValue = new \DateTime($nodeValue);
+            return $cellValue;
+        } catch (\Exception $e) {
+            return null;
+        }
+    }
+
+    /**
+     * Returns the (unescaped) correctly marshalled, cell value associated to the given XML node.
      *
      * @param \DOMNode $node
      * @param \Box\Spout\Common\Escaper\XLSX $escaper
@@ -232,47 +348,30 @@ class XLSX extends AbstractReader
      */
     protected function getCellValue($node, $escaper)
     {
-        $cellValue = '';
-
         // Default cell type is "n"
         $cellType = $node->getAttribute('t') ?: 'n';
-
+        $vNodeValue = $this->getVNodeValue($node);
-        if ($cellType === 'inlineStr') {
+        if ( ($vNodeValue === "") && ($cellType !== self::CELL_TYPE_INLINE_STRING) ) {
-            // inline strings are formatted this way:
+            return $vNodeValue;
-            // <c r="A1" t="inlineStr"><is><t>[INLINE_STRING]</t></is></c>
-            $tNode = $node->getElementsByTagName('t')->item(0);
-            $escapedCellValue = trim($tNode->nodeValue);
-            $cellValue = $escaper->unescape($escapedCellValue);
-        } else {
-            // all other cell types should have a "v" tag containing the value.
-            // if not, the returned value should be empty string.
-            $vNode = $node->getElementsByTagName('v')->item(0);
-
-            if ($vNode !== null) {
-                if ($cellType === 's') {
-                    // shared strings are formatted this way:
-                    // <c r="A1" t="s"><v>[SHARED_STRING_INDEX]</v></c>
-                    $sharedStringIndex = intval($vNode->nodeValue);
-                    $escapedCellValue = $this->sharedStringsHelper->getStringAtIndex($sharedStringIndex);
-                    $cellValue = $escaper->unescape($escapedCellValue);
-                } else if ($cellType === 'b') {
-                    // !! is similar to boolval()
-                    $cellValue = !!$vNode->nodeValue;
-                } else if ($cellType === 'n') {
-                    $nodeValue = $vNode->nodeValue;
-                    $cellValue = is_int($nodeValue) ? intval($nodeValue) : floatval($nodeValue);
-                } else if ($cellType === 'd') {
-                    $cellValue = new \DateTime($vNode->nodeValue);
-                } else if ($cellType === 'e') {
-                    $cellValue = null;
-                } else if ($cellType === 'str') {
-                    $escapedCellValue = trim($vNode->nodeValue);
-                    $cellValue = $escaper->unescape($escapedCellValue);
-                }
-            }
         }
 
-        return $cellValue;
+        switch ($cellType)
+        {
+            case self::CELL_TYPE_INLINE_STRING:
+                return $this->formatInlineStringCellValue($node, $escaper);
+            case self::CELL_TYPE_SHARED_STRING:
+                return $this->formatSharedStringCellValue($vNodeValue, $escaper);
+            case self::CELL_TYPE_STR:
+                return $this->formatStrCellValue($vNodeValue, $escaper);
+            case self::CELL_TYPE_BOOLEAN:
+                return $this->formatBooleanCellValue($vNodeValue);
+            case self::CELL_TYPE_NUMERIC:
+                return $this->formatNumericCellValue($vNodeValue);
+            case self::CELL_TYPE_DATE:
+                return $this->formatDateCellValue($vNodeValue);
+            default:
+                return null;
+        }
     }
 
     /**

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

@@ -36,7 +36,7 @@ EOD;
      */
     public function __construct($xlFolder)
     {
-        $sharedStringsFilePath = $xlFolder . DIRECTORY_SEPARATOR . self::SHARED_STRINGS_FILE_NAME;
+        $sharedStringsFilePath = $xlFolder . '/' . self::SHARED_STRINGS_FILE_NAME;
         $this->sharedStringsFilePointer = fopen($sharedStringsFilePath, 'w');
 
         $this->throwIfSharedStringsFilePointerIsNotAvailable();

src/Spout/Writer/Helper/XLSX/ZipHelper.php

@@ -33,18 +33,30 @@ class ZipHelper
      */
     protected function addFolderToZip($zip, $folderPath)
     {
-        $folderRealPath = realpath($folderPath) . DIRECTORY_SEPARATOR;
+        $folderRealPath = $this->getNormalizedRealPath($folderPath) . '/';
         $itemIterator = new \RecursiveIteratorIterator(new \RecursiveDirectoryIterator($folderPath, \RecursiveDirectoryIterator::SKIP_DOTS), \RecursiveIteratorIterator::SELF_FIRST);
 
         foreach ($itemIterator as $itemInfo) {
-            $itemRealPath = realpath($itemInfo->getPathname());
+            $itemRealPath = $this->getNormalizedRealPath($itemInfo->getPathname());
             $itemLocalPath = str_replace($folderRealPath, '', $itemRealPath);
 
             if ($itemInfo->isFile()) {
-                $zip->addFile($itemInfo->getPathname(), $itemLocalPath);
+                $zip->addFile($itemRealPath, $itemLocalPath);
             } else if ($itemInfo->isDir()) {
                 $zip->addEmptyDir($itemLocalPath);
             }
         }
     }
+
+    /**
+     * Returns canonicalized absolute pathname, containing only forward slashes.
+     *
+     * @param string $path Path to normalize
+     * @return string Normalized and canonicalized path
+     */
+    protected function getNormalizedRealPath($path)
+    {
+        $realPath = realpath($path);
+        return str_replace(DIRECTORY_SEPARATOR, '/', $realPath);
+    }
 }

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

@@ -56,7 +56,7 @@ EOD;
 
         $this->stringsEscaper = new \Box\Spout\Common\Escaper\XLSX();
 
-        $this->worksheetFilePath = $worksheetFilesFolder . DIRECTORY_SEPARATOR . strtolower($this->externalSheet->getName()) . '.xml';
+        $this->worksheetFilePath = $worksheetFilesFolder . '/' . strtolower($this->externalSheet->getName()) . '.xml';
         $this->startSheet();
     }
 

tests/Spout/Reader/XLSXTest.php

@@ -129,6 +129,8 @@ class XLSXTest extends \PHPUnit_Framework_TestCase
                 \DateTime::createFromFormat('Y-m-d H:i:s', '2015-06-01 00:00:00'),
                 10, 10.43,
                 null,
+                'weird string', // valid 'str' string
+                null, // invalid date
             ],
             ['', '', '', '', '', '', '', '', ''],
         ];
@@ -250,7 +252,7 @@ class XLSXTest extends \PHPUnit_Framework_TestCase
     {
         $allRows = $this->getAllRowsForFile('billion_laughs_test_file.xlsx');
 
-        $expectedMaxMemoryUsage = 20 * 1024 * 1024; // 20MB
+        $expectedMaxMemoryUsage = 30 * 1024 * 1024; // 30MB
         $this->assertLessThan($expectedMaxMemoryUsage, memory_get_peak_usage(true), 'Entities should not be expanded and therefore consume all the memory.');
 
         $expectedFirstRow = ['s1--A1', 's1--B1', 's1--C1', 's1--D1', 's1--E1'];

tests/Spout/Writer/SheetTest.php

@@ -90,18 +90,9 @@ class SheetTest extends \PHPUnit_Framework_TestCase
      */
     private function assertSheetNameEquals($expectedName, $resourcePath, $message = '')
     {
-        $pathToWorkbookFile = $resourcePath . $this->normalizePath('#xl/workbook.xml');
+        $pathToWorkbookFile = $resourcePath . '#xl/workbook.xml';
         $xmlContents = file_get_contents('zip://' . $pathToWorkbookFile);
 
         $this->assertContains('<sheet name="' . $expectedName . '"', $xmlContents, $message);
     }
-
-    /**
-     * @param string $path
-     * @return string The path with the correct directory separators, as defined for the current platform
-     */
-    private function normalizePath($path)
-    {
-        return str_replace('/', DIRECTORY_SEPARATOR, $path);
-    }
 }

tests/Spout/Writer/XLSXTest.php

@@ -408,7 +408,7 @@ class XLSXTest extends \PHPUnit_Framework_TestCase
     private function assertInlineDataWasWrittenToSheet($fileName, $sheetIndex, $inlineData, $message = '')
     {
         $resourcePath = $this->getGeneratedResourcePath($fileName);
-        $pathToSheetFile = $resourcePath . $this->normalizePath('#xl/worksheets/sheet' . $sheetIndex . '.xml');
+        $pathToSheetFile = $resourcePath . '#xl/worksheets/sheet' . $sheetIndex . '.xml';
         $xmlContents = file_get_contents('zip://' . $pathToSheetFile);
 
         $this->assertContains((string)$inlineData, $xmlContents, $message);
@@ -424,7 +424,7 @@ class XLSXTest extends \PHPUnit_Framework_TestCase
     private function assertInlineDataWasNotWrittenToSheet($fileName, $sheetIndex, $inlineData, $message = '')
     {
         $resourcePath = $this->getGeneratedResourcePath($fileName);
-        $pathToSheetFile = $resourcePath . $this->normalizePath('#xl/worksheets/sheet' . $sheetIndex . '.xml');
+        $pathToSheetFile = $resourcePath . '#xl/worksheets/sheet' . $sheetIndex . '.xml';
         $xmlContents = file_get_contents('zip://' . $pathToSheetFile);
 
         $this->assertNotContains((string)$inlineData, $xmlContents, $message);
@@ -439,18 +439,9 @@ class XLSXTest extends \PHPUnit_Framework_TestCase
     private function assertSharedStringWasWritten($fileName, $sharedString, $message = '')
     {
         $resourcePath = $this->getGeneratedResourcePath($fileName);
-        $pathToSharedStringsFile = $resourcePath . $this->normalizePath('#xl/sharedStrings.xml');
+        $pathToSharedStringsFile = $resourcePath . '#xl/sharedStrings.xml';
         $xmlContents = file_get_contents('zip://' . $pathToSharedStringsFile);
 
         $this->assertContains($sharedString, $xmlContents, $message);
     }
-
-    /**
-     * @param string $path
-     * @return string The path with the correct directory separators, as defined for the current platform
-     */
-    private function normalizePath($path)
-    {
-        return str_replace('/', DIRECTORY_SEPARATOR, $path);
-    }
 }