Test filtered columns are still excluded

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.

src/PhpSpreadsheet/Reader/Xlsx.php

@@ -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();
                             }

src/PhpSpreadsheet/Reader/Xlsx/ColumnAndRowAttributes.php

@@ -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)

tests/PhpSpreadsheetTests/Functional/ColumnWidthTest.php

@@ -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);
     }
 }

tests/PhpSpreadsheetTests/Functional/RowHeightTest.php

@@ -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());
+    }
+}