Skip to content

Commit

Permalink
Shared strings table without uniqueCount and count should work (box#269)
Browse files Browse the repository at this point in the history
Use file based strategy in this case
  • Loading branch information
adrilo authored Jul 11, 2016
1 parent ffea887 commit a8eb7ad
Show file tree
Hide file tree
Showing 5 changed files with 26 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ public static function getInstance()
* Returns the best caching strategy, given the number of unique shared strings
* and the amount of memory available.
*
* @param int $sharedStringsUniqueCount Number of unique shared strings
* @param int|null $sharedStringsUniqueCount Number of unique shared strings (NULL if unknown)
* @param string|void $tempFolder Temporary folder where the temporary files to store shared strings will be stored
* @return CachingStrategyInterface The best caching strategy
*/
Expand All @@ -95,11 +95,16 @@ public function getBestCachingStrategy($sharedStringsUniqueCount, $tempFolder =
* Returns whether it is safe to use in-memory caching, given the number of unique shared strings
* and the amount of memory available.
*
* @param int $sharedStringsUniqueCount Number of unique shared strings
* @param int|null $sharedStringsUniqueCount Number of unique shared strings (NULL if unknown)
* @return bool
*/
protected function isInMemoryStrategyUsageSafe($sharedStringsUniqueCount)
{
// if the number of shared strings in unknown, do not use "in memory" strategy
if ($sharedStringsUniqueCount === null) {
return false;
}

$memoryAvailable = $this->getMemoryLimitInKB();

if ($memoryAvailable === -1) {
Expand Down
6 changes: 3 additions & 3 deletions src/Spout/Reader/XLSX/Helper/SharedStringsHelper.php
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ protected function getSharedStringsFilePath()
* Returns the shared strings unique count, as specified in <sst> tag.
*
* @param \Box\Spout\Reader\Wrapper\XMLReader $xmlReader XMLReader instance
* @return int Number of unique shared strings in the sharedStrings.xml file
* @return int|null Number of unique shared strings in the sharedStrings.xml file
* @throws \Box\Spout\Common\Exception\IOException If sharedStrings.xml is invalid and can't be read
*/
protected function getSharedStringsUniqueCount($xmlReader)
Expand All @@ -167,13 +167,13 @@ protected function getSharedStringsUniqueCount($xmlReader)
$uniqueCount = $xmlReader->getAttribute('count');
}

return intval($uniqueCount);
return ($uniqueCount !== null) ? intval($uniqueCount) : null;
}

/**
* Returns the best shared strings caching strategy.
*
* @param int $sharedStringsUniqueCount
* @param int|null $sharedStringsUniqueCount Number of unique shared strings (NULL if unknown)
* @return CachingStrategyInterface
*/
protected function getBestSharedStringsCachingStrategy($sharedStringsUniqueCount)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ class CachingStrategyFactoryTest extends \PHPUnit_Framework_TestCase
public function dataProviderForTestGetBestCachingStrategy()
{
return [
[null, -1, 'FileBasedStrategy'],
[CachingStrategyFactory::MAX_NUM_STRINGS_PER_TEMP_FILE, -1, 'FileBasedStrategy'],
[CachingStrategyFactory::MAX_NUM_STRINGS_PER_TEMP_FILE + 10, -1, 'FileBasedStrategy'],
[CachingStrategyFactory::MAX_NUM_STRINGS_PER_TEMP_FILE - 10, -1, 'InMemoryStrategy'],
Expand All @@ -27,7 +28,7 @@ public function dataProviderForTestGetBestCachingStrategy()
/**
* @dataProvider dataProviderForTestGetBestCachingStrategy
*
* @param int $sharedStringsUniqueCount
* @param int|null $sharedStringsUniqueCount
* @param int $memoryLimitInKB
* @param string $expectedStrategyClassName
* @return void
Expand Down
14 changes: 14 additions & 0 deletions tests/Spout/Reader/XLSX/ReaderTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,20 @@ public function testReadShouldSupportSheetWithSharedStringsMissingUniqueCountAtt
$this->assertEquals($expectedRows, $allRows);
}

/**
* @return void
*/
public function testReadShouldSupportSheetWithSharedStringsMissingUniqueCountAndCountAttributes()
{
$allRows = $this->getAllRowsForFile('one_sheet_with_shared_strings_missing_unique_count_and_count.xlsx');

$expectedRows = [
['s1--A1', 's1--B1'],
['s1--A2', 's1--B2'],
];
$this->assertEquals($expectedRows, $allRows);
}

/**
* @return void
*/
Expand Down
Binary file not shown.

0 comments on commit a8eb7ad

Please sign in to comment.