diff --git a/CHANGELOG.md b/CHANGELOG.md index 2d7b4ebe..d96323dd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,7 +14,7 @@ and this project adheres to [Semantic Versioning](https://semver.org). ### Changed -- Nothing. +- Improve Coverage for ODS Reader [#1545](https://github.com/phpoffice/phpspreadsheet/pull/1544) ### Deprecated diff --git a/src/PhpSpreadsheet/Reader/Ods.php b/src/PhpSpreadsheet/Reader/Ods.php index 4f164b6f..b2a6a759 100644 --- a/src/PhpSpreadsheet/Reader/Ods.php +++ b/src/PhpSpreadsheet/Reader/Ods.php @@ -11,6 +11,7 @@ use DOMNode; use PhpOffice\PhpSpreadsheet\Calculation\Calculation; use PhpOffice\PhpSpreadsheet\Cell\Coordinate; use PhpOffice\PhpSpreadsheet\Cell\DataType; +use PhpOffice\PhpSpreadsheet\Reader\Exception as ReaderException; use PhpOffice\PhpSpreadsheet\Reader\Ods\PageSettings; use PhpOffice\PhpSpreadsheet\Reader\Ods\Properties as DocumentProperties; use PhpOffice\PhpSpreadsheet\Reader\Security\XmlScanner; @@ -76,11 +77,9 @@ class Ods extends BaseReader } $zip->close(); - - return $mimeType === 'application/vnd.oasis.opendocument.spreadsheet'; } - return false; + return $mimeType === 'application/vnd.oasis.opendocument.spreadsheet'; } /** @@ -95,8 +94,8 @@ class Ods extends BaseReader File::assertFile($pFilename); $zip = new ZipArchive(); - if (!$zip->open($pFilename)) { - throw new Exception('Could not open ' . $pFilename . ' for reading! Error opening file.'); + if ($zip->open($pFilename) !== true) { + throw new ReaderException('Could not open ' . $pFilename . ' for reading! Error opening file.'); } $worksheetNames = []; @@ -149,8 +148,8 @@ class Ods extends BaseReader $worksheetInfo = []; $zip = new ZipArchive(); - if (!$zip->open($pFilename)) { - throw new Exception('Could not open ' . $pFilename . ' for reading! Error opening file.'); + if ($zip->open($pFilename) !== true) { + throw new ReaderException('Could not open ' . $pFilename . ' for reading! Error opening file.'); } $xml = new XMLReader(); @@ -198,18 +197,18 @@ class Ods extends BaseReader // Step into the row $xml->read(); do { + $doread = true; if ($xml->name == 'table:table-cell' && $xml->nodeType == XMLReader::ELEMENT) { if (!$xml->isEmptyElement) { ++$currCells; $xml->next(); - } else { - $xml->read(); + $doread = false; } } elseif ($xml->name == 'table:covered-table-cell' && $xml->nodeType == XMLReader::ELEMENT) { $mergeSize = $xml->getAttribute('table:number-columns-repeated'); $currCells += (int) $mergeSize; - $xml->read(); - } else { + } + if ($doread) { $xml->read(); } } while ($xml->name != 'table:table-row'); @@ -258,13 +257,13 @@ class Ods extends BaseReader $GMT = new DateTimeZone('UTC'); $zip = new ZipArchive(); - if (!$zip->open($pFilename)) { + if ($zip->open($pFilename) !== true) { throw new Exception("Could not open {$pFilename} for reading! Error opening file."); } // Meta - $xml = simplexml_load_string( + $xml = @simplexml_load_string( $this->securityScanner->scan($zip->getFromName('meta.xml')), 'SimpleXMLElement', Settings::getLibXmlLoaderOptions() @@ -465,9 +464,10 @@ class Ods extends BaseReader $type = DataType::TYPE_NUMERIC; $dataValue = (float) $cellData->getAttributeNS($officeNs, 'value'); - if (floor($dataValue) == $dataValue) { - $dataValue = (int) $dataValue; - } + // percentage should always be float + //if (floor($dataValue) == $dataValue) { + // $dataValue = (int) $dataValue; + //} $formatting = NumberFormat::FORMAT_PERCENTAGE_00; break; @@ -488,8 +488,6 @@ class Ods extends BaseReader if (floor($dataValue) == $dataValue) { if ($dataValue == (int) $dataValue) { $dataValue = (int) $dataValue; - } else { - $dataValue = (float) $dataValue; } } diff --git a/src/PhpSpreadsheet/Reader/Ods/Properties.php b/src/PhpSpreadsheet/Reader/Ods/Properties.php index c0b309a9..d0a45e6a 100644 --- a/src/PhpSpreadsheet/Reader/Ods/Properties.php +++ b/src/PhpSpreadsheet/Reader/Ods/Properties.php @@ -54,15 +54,11 @@ class Properties $docProps->setLastModifiedBy($propertyValue); break; - case 'creation-date': + case 'date': $creationDate = strtotime($propertyValue); $docProps->setCreated($creationDate); $docProps->setModified($creationDate); - break; - case 'keyword': - $docProps->setKeywords($propertyValue); - break; case 'description': $docProps->setDescription($propertyValue); diff --git a/tests/PhpSpreadsheetTests/Reader/Ods/OdsInfoTest.php b/tests/PhpSpreadsheetTests/Reader/Ods/OdsInfoTest.php new file mode 100644 index 00000000..1de4252f --- /dev/null +++ b/tests/PhpSpreadsheetTests/Reader/Ods/OdsInfoTest.php @@ -0,0 +1,110 @@ +listWorksheetNames($filename)); + } + + public function testNoMimeType(): void + { + $filename = 'tests/data/Reader/Ods/nomimetype.ods'; + + // Load into this instance + $reader = new Ods(); + + self::assertTrue($reader->canRead($filename)); + } + + public function testReadBadFileProperties(): void + { + $this->expectException(ReaderException::class); + + // Load into this instance + $reader = new Ods(); + + // Test "listWorksheetNames" method + + self::assertEquals([ + 'Sheet1', + 'Second Sheet', + ], $reader->listWorksheetNames(__FILE__)); + } + + public function testReadFileInfo(): void + { + $filename = 'tests/data/Reader/Ods/data.ods'; + + // Load into this instance + $reader = new Ods(); + + // Test "listWorksheetNames" method + + $wsinfo = $reader->listWorkSheetInfo($filename); + self::assertEquals([ + [ + 'worksheetName' => 'Sheet1', + 'lastColumnLetter' => 'C', + 'lastColumnIndex' => 2, + 'totalRows' => 12, + 'totalColumns' => 3, + ], + [ + 'worksheetName' => 'Second Sheet', + 'lastColumnLetter' => 'A', + 'lastColumnIndex' => 0, + 'totalRows' => 2, + 'totalColumns' => 1, + ], + ], $wsinfo); + } + + public function testReadBadFileInfo(): void + { + $this->expectException(ReaderException::class); + $filename = __FILE__; + + // Load into this instance + $reader = new Ods(); + + // Test "listWorksheetNames" method + + $wsinfo = $reader->listWorkSheetInfo($filename); + self::assertEquals([ + [ + 'worksheetName' => 'Sheet1', + 'lastColumnLetter' => 'C', + 'lastColumnIndex' => 2, + 'totalRows' => 11, + 'totalColumns' => 3, + ], + [ + 'worksheetName' => 'Second Sheet', + 'lastColumnLetter' => 'A', + 'lastColumnIndex' => 0, + 'totalRows' => 2, + 'totalColumns' => 1, + ], + ], $wsinfo); + } +} diff --git a/tests/PhpSpreadsheetTests/Reader/OdsTest.php b/tests/PhpSpreadsheetTests/Reader/Ods/OdsTest.php similarity index 83% rename from tests/PhpSpreadsheetTests/Reader/OdsTest.php rename to tests/PhpSpreadsheetTests/Reader/Ods/OdsTest.php index 0aeb5570..8be1aa7c 100644 --- a/tests/PhpSpreadsheetTests/Reader/OdsTest.php +++ b/tests/PhpSpreadsheetTests/Reader/Ods/OdsTest.php @@ -1,9 +1,10 @@ spreadsheetData = $reader->loadIntoExisting($filename, new Spreadsheet()); + $this->spreadsheetData = $reader->load($filename); } return $this->spreadsheetData; } - public function testReadFileProperties(): void + public function testLoadWorksheets(): void + { + $spreadsheet = $this->loadDataFile(); + + self::assertInstanceOf('PhpOffice\PhpSpreadsheet\Spreadsheet', $spreadsheet); + + self::assertEquals(2, $spreadsheet->getSheetCount()); + + $firstSheet = $spreadsheet->getSheet(0); + self::assertInstanceOf('PhpOffice\PhpSpreadsheet\Worksheet\Worksheet', $firstSheet); + + $secondSheet = $spreadsheet->getSheet(1); + self::assertInstanceOf('PhpOffice\PhpSpreadsheet\Worksheet\Worksheet', $secondSheet); + self::assertEquals('Sheet1', $spreadsheet->getSheet(0)->getTitle()); + self::assertEquals('Second Sheet', $spreadsheet->getSheet(1)->getTitle()); + } + + public function testLoadOneWorksheet(): void { $filename = 'tests/data/Reader/Ods/data.ods'; // Load into this instance $reader = new Ods(); + $reader->setLoadSheetsOnly(['Sheet1']); + $spreadsheet = $reader->load($filename); - // Test "listWorksheetNames" method + self::assertEquals(1, $spreadsheet->getSheetCount()); - self::assertEquals([ - 'Sheet1', - 'Second Sheet', - ], $reader->listWorksheetNames($filename)); + self::assertEquals('Sheet1', $spreadsheet->getSheet(0)->getTitle()); } - public function testLoadWorksheets(): void + public function testLoadBadFile(): void { - $spreadsheet = $this->loadDataFile(); + $this->expectException(ReaderException::class); + $reader = new Ods(); + $spreadsheet = $reader->load(__FILE__); + + self::assertInstanceOf('PhpOffice\PhpSpreadsheet\Spreadsheet', $spreadsheet); + + self::assertEquals(2, $spreadsheet->getSheetCount()); + + $firstSheet = $spreadsheet->getSheet(0); + self::assertInstanceOf('PhpOffice\PhpSpreadsheet\Worksheet\Worksheet', $firstSheet); + + $secondSheet = $spreadsheet->getSheet(1); + self::assertInstanceOf('PhpOffice\PhpSpreadsheet\Worksheet\Worksheet', $secondSheet); + } + + public function testLoadCorruptFile(): void + { + $this->expectException(ReaderException::class); + $filename = 'tests/data/Reader/Ods/corruptMeta.ods'; + $reader = new Ods(); + $spreadsheet = $reader->load($filename); self::assertInstanceOf('PhpOffice\PhpSpreadsheet\Spreadsheet', $spreadsheet); diff --git a/tests/data/Reader/Ods/corruptMeta.ods b/tests/data/Reader/Ods/corruptMeta.ods new file mode 100644 index 00000000..146befd9 Binary files /dev/null and b/tests/data/Reader/Ods/corruptMeta.ods differ diff --git a/tests/data/Reader/Ods/data.ods b/tests/data/Reader/Ods/data.ods index 3171eb6b..2aaad631 100644 Binary files a/tests/data/Reader/Ods/data.ods and b/tests/data/Reader/Ods/data.ods differ diff --git a/tests/data/Reader/Ods/nomimetype.ods b/tests/data/Reader/Ods/nomimetype.ods new file mode 100644 index 00000000..26d53847 Binary files /dev/null and b/tests/data/Reader/Ods/nomimetype.ods differ