Compare commits

..

4 Commits

Author SHA1 Message Date
Alex Wright 0f4d11577a Test filtered columns are still excluded 2020-12-13 19:38:06 +01:00
Alex Wright e1018e2330 Use worksheet->getHighestColumn/Row for isFiltered
There may very well be a good reason the row/col attributes are loaded
first. This is passing the test suite though and I think makes more
sense than iterating over the Row/Col attrs to get the Col/Row
isFiltered bool.
2020-12-13 19:38:06 +01:00
Alex Wright f366c0f257 Change how ReadFilters are interpreted 2020-12-13 19:38:06 +01:00
Alex Wright 37bf44e9db Test reading Xlsx column/row attrs with ReadFilter
When reading an Xlsx file using a ReadFilter it is posible for
`ColumnAndRowAttributes::load()` to (I think) erroneously skip loading
of column or row attributes.

This is because `isFilteredColumn` and `isFilteredRow` will return early
with a true (ie the column or row should be considered filtered and
their attrbutes discarded) at the first false returned from the
ReadFilter. All rows for a column must past the filter for the loop to
return false. I've also created the same test of column widths for row
heights.

 * ColumnWidthTest.php
   Added test with a read filter.
 * RowHeightTest.php
   Mirrored column wdith tests for row height
2020-12-13 19:37:55 +01:00
4 changed files with 160 additions and 31 deletions

View File

@ -630,19 +630,6 @@ class Xlsx extends BaseReader
$docSheet->setSheetState((string) $eleSheet['state']);
}
if ($xmlSheet) {
if (isset($xmlSheet->sheetViews, $xmlSheet->sheetViews->sheetView)) {
$sheetViews = new SheetViews($xmlSheet->sheetViews->sheetView, $docSheet);
$sheetViews->load();
}
$sheetViewOptions = new SheetViewOptions($docSheet, $xmlSheet);
$sheetViewOptions->load($this->getReadDataOnly());
(new ColumnAndRowAttributes($docSheet, $xmlSheet))
->load($this->getReadFilter(), $this->getReadDataOnly());
}
if ($xmlSheet && $xmlSheet->sheetData && $xmlSheet->sheetData->row) {
$cIndex = 1; // Cell Start from 1
foreach ($xmlSheet->sheetData->row as $row) {
@ -756,6 +743,19 @@ class Xlsx extends BaseReader
}
}
if ($xmlSheet) {
if (isset($xmlSheet->sheetViews, $xmlSheet->sheetViews->sheetView)) {
$sheetViews = new SheetViews($xmlSheet->sheetViews->sheetView, $docSheet);
$sheetViews->load();
}
$sheetViewOptions = new SheetViewOptions($docSheet, $xmlSheet);
$sheetViewOptions->load($this->getReadDataOnly());
(new ColumnAndRowAttributes($docSheet, $xmlSheet))
->load($this->getReadFilter(), $this->getReadDataOnly());
}
if (!$this->readDataOnly && $xmlSheet && $xmlSheet->conditionalFormatting) {
(new ConditionalStyles($docSheet, $xmlSheet, $dxfs))->load();
}

View File

@ -96,7 +96,7 @@ class ColumnAndRowAttributes extends BaseParserClass
foreach ($columnsAttributes as $columnCoordinate => $columnAttributes) {
if (
$readFilter === null ||
!$this->isFilteredColumn($readFilter, $columnCoordinate, $rowsAttributes)
!$this->isFilteredColumn($readFilter, $columnCoordinate)
) {
if (!isset($columnsAttributesAreSet[$columnCoordinate])) {
$this->setColumnAttributes($columnCoordinate, $columnAttributes);
@ -109,7 +109,7 @@ class ColumnAndRowAttributes extends BaseParserClass
foreach ($rowsAttributes as $rowCoordinate => $rowAttributes) {
if (
$readFilter === null ||
!$this->isFilteredRow($readFilter, $rowCoordinate, $columnsAttributes)
!$this->isFilteredRow($readFilter, $rowCoordinate)
) {
if (!isset($rowsAttributesAreSet[$rowCoordinate])) {
$this->setRowAttributes($rowCoordinate, $rowAttributes);
@ -119,15 +119,14 @@ class ColumnAndRowAttributes extends BaseParserClass
}
}
private function isFilteredColumn(IReadFilter $readFilter, $columnCoordinate, array $rowsAttributes)
private function isFilteredColumn(IReadFilter $readFilter, $columnCoordinate)
{
foreach ($rowsAttributes as $rowCoordinate => $rowAttributes) {
if (!$readFilter->readCell($columnCoordinate, $rowCoordinate, $this->worksheet->getTitle())) {
return true;
for ($rowCoordinate = 1; $rowCoordinate <= $this->worksheet->getHighestRow(); $rowCoordinate++) {
if ($readFilter->readCell($columnCoordinate, $rowCoordinate, $this->worksheet->getTitle())) {
return false;
}
}
return false;
return true;
}
private function readColumnAttributes(SimpleXMLElement $worksheetCols, $readDataOnly)
@ -171,15 +170,15 @@ class ColumnAndRowAttributes extends BaseParserClass
return $columnAttributes;
}
private function isFilteredRow(IReadFilter $readFilter, $rowCoordinate, array $columnsAttributes)
private function isFilteredRow(IReadFilter $readFilter, $rowCoordinate)
{
foreach ($columnsAttributes as $columnCoordinate => $columnAttributes) {
if (!$readFilter->readCell($columnCoordinate, $rowCoordinate, $this->worksheet->getTitle())) {
return true;
$lastColumnIndex = Coordinate::columnIndexFromString($this->worksheet->getHighestColumn());
for ($columnIndex = 1; $columnIndex <= $lastColumnIndex; $columnIndex++) {
if ($readFilter->readCell(Coordinate::stringFromColumnIndex($columnIndex), $rowCoordinate)) {
return false;
}
}
return false;
return true;
}
private function readRowAttributes(SimpleXMLElement $worksheetRow, $readDataOnly)

View File

@ -2,6 +2,7 @@
namespace PhpOffice\PhpSpreadsheetTests\Functional;
use PhpOffice\PhpSpreadsheet\Reader\IReadFilter;
use PhpOffice\PhpSpreadsheet\Spreadsheet;
class ColumnWidthTest extends AbstractFunctional
@ -31,13 +32,62 @@ class ColumnWidthTest extends AbstractFunctional
$this->assertColumn($reloadedSpreadsheet);
}
private function assertColumn(Spreadsheet $spreadsheet): void
/**
* @dataProvider providerFormats
*
* @param $format
*/
public function testReadColumnWidthWithReadFilter($format): void
{
// Same test with a read filter removing a single row
$spreadsheet = new Spreadsheet();
$sheet = $spreadsheet->getActiveSheet();
foreach (range(1, 5) as $row) {
$sheet->setCellValue("A{$row}", 'Hello');
$sheet->setCellValue("B{$row}", 'World !');
}
// isFilteredColumn iterates rowAttributes when calling the read filter
$sheet->getRowDimension(5)->setRowHeight(10);
$sheet->getColumnDimension('A')->setWidth(20);
$sheet->getColumnDimension('B')->setWidth(30);
$this->assertColumn($spreadsheet);
$this->assertColumn($spreadsheet, 'B', 30);
// A reader-customeiser closure and ReadFilter implementation that skips rows >4
$readerCustomizer = function ($reader) {
$readFilterStub = $this->createMock(IReadFilter::class);
$readFilterStub->method('readCell')
->willReturnCallback(function ($column, $row, $worksheetName = '') {
if ($column == 'B') {
return false;
}
return $row <= 4;
});
$reader->setReadFilter($readFilterStub);
};
// Save and reload a filtered set and assert the same width
$reloadedSpreadsheet = $this->writeAndReload($spreadsheet, $format, $readerCustomizer);
$this->assertColumn($reloadedSpreadsheet);
$this->assertNotColumn($reloadedSpreadsheet, 'B', 30);
}
private function assertColumn(Spreadsheet $spreadsheet, $column = 'A', $expected = 20): void
{
$sheet = $spreadsheet->getActiveSheet();
$columnDimensions = $sheet->getColumnDimensions();
self::assertArrayHasKey('A', $columnDimensions);
$column = array_shift($columnDimensions);
self::assertEquals(20, $column->getWidth());
self::assertArrayHasKey($column, $columnDimensions);
self::assertEquals($expected, $columnDimensions[$column]->getWidth());
}
private function assertNotColumn(Spreadsheet $spreadsheet, $column = 'A', $expected = 20): void
{
$sheet = $spreadsheet->getActiveSheet();
$columnDimensions = $sheet->getColumnDimensions();
self::assertArrayNotHasKey($column, $columnDimensions);
}
}

View File

@ -0,0 +1,80 @@
<?php
namespace PhpOffice\PhpSpreadsheetTests\Functional;
use PhpOffice\PhpSpreadsheet\Cell\Coordinate;
use PhpOffice\PhpSpreadsheet\Reader\IReadFilter;
use PhpOffice\PhpSpreadsheet\Spreadsheet;
class RowHeightTest extends AbstractFunctional
{
public function providerFormats()
{
return [
['Xlsx'],
];
}
/**
* @dataProvider providerFormats
*
* @param $format
*/
public function testReadRowHeight($format): void
{
// create new sheet with column width
$spreadsheet = new Spreadsheet();
$sheet = $spreadsheet->getActiveSheet();
$sheet->setCellValue('A1', 'Hello World !');
$sheet->getRowDimension('1')->setRowHeight(20);
$this->assertRow($spreadsheet);
$reloadedSpreadsheet = $this->writeAndReload($spreadsheet, $format);
$this->assertRow($reloadedSpreadsheet);
}
/**
* @dataProvider providerFormats
*
* @param $format
*/
public function testReadRowHeightWithReadFilter($format): void
{
// Same test with a read filter removing a single row
$spreadsheet = new Spreadsheet();
$sheet = $spreadsheet->getActiveSheet();
foreach (range(1, 5) as $columnIndex) {
$sheet->setCellValue(Coordinate::stringFromColumnIndex($columnIndex) . '1', 'Hello World !');
}
// isFilteredRow iterates columnAttributes when calling the read filter
$sheet->getColumnDimension('E')->setWidth(64);
$sheet->getRowDimension('1')->setRowHeight(20);
$this->assertRow($spreadsheet);
// A reader-customeiser closure and ReadFilter implementation that skips column 'E'
$readerCustomizer = function ($reader) {
$readFilterStub = $this->createMock(IReadFilter::class);
$readFilterStub->method('readCell')
->willReturnCallback(function ($column, $row, $worksheetName = '') {
return $column !== 'E';
});
$reader->setReadFilter($readFilterStub);
};
// Save and reload a filtered set and assert the same width
$reloadedSpreadsheet = $this->writeAndReload($spreadsheet, $format, $readerCustomizer);
$this->assertRow($reloadedSpreadsheet);
}
private function assertRow(Spreadsheet $spreadsheet): void
{
$sheet = $spreadsheet->getActiveSheet();
$rowDimensions = $sheet->getRowDimensions();
self::assertArrayHasKey('1', $rowDimensions);
$row = array_shift($rowDimensions);
self::assertEquals(20, $row->getRowHeight());
}
}