Xlsx Reader External Data Validations Flag Missing (#3078)

* Xlsx Reader External Data Validations Flag Missing

Fix #2677. This PR supersedes #2679, written by @technghiath, which lacks tests, and probably doesn't solve the problem entirely. The code causing the problem appears to be the last remnant in Xlsx Reader which calls `children` using a namespace prefix rather than a namespace. That is changed, and tests are added where the tag is unexpectedly missing, and also where it uses a non-standard namespace prefix.

* Scrutinizer

Reports 1 "new" error. It isn't, but fix it anyhow.

* Fix One Existing Scrutinizer Problem

Only remaining problem in Reader/Xlsx.

Author: oleibman

src/PhpSpreadsheet/Reader/Xlsx.php

@@ -469,6 +469,7 @@ protected function loadSpreadsheetFromFile(string $filename): Spreadsheet
         $rels = $this->loadZip(self::INITIAL_FILE, Namespaces::RELATIONSHIPS);
 
         $propertyReader = new PropertyReader($this->securityScanner, $excel->getProperties());
+        $chartDetails = [];
         foreach ($rels->Relationship as $relx) {
             $rel = self::getAttributes($relx);
             $relTarget = (string) $rel['Target'];
@@ -929,13 +930,16 @@ protected function loadSpreadsheetFromFile(string $filename): Spreadsheet
                                     $xmlSheet->addChild('dataValidations');
                                 }
 
-                                foreach ($xmlSheet->extLst->ext->children('x14', true)->dataValidations->dataValidation as $item) {
+                                foreach ($xmlSheet->extLst->ext->children(Namespaces::DATA_VALIDATIONS1)->dataValidations->dataValidation as $item) {
+                                    $item = self::testSimpleXml($item);
                                     $node = self::testSimpleXml($xmlSheet->dataValidations)->addChild('dataValidation');
                                     foreach ($item->attributes() ?? [] as $attr) {
                                         $node->addAttribute($attr->getName(), $attr);
                                     }
-                                    $node->addAttribute('sqref', $item->children('xm', true)->sqref);
-                                    $node->addChild('formula1', $item->formula1->children('xm', true)->f);
+                                    $node->addAttribute('sqref', $item->children(Namespaces::DATA_VALIDATIONS2)->sqref);
+                                    if (isset($item->formula1)) {
+                                        $node->addChild('formula1', $item->formula1->children(Namespaces::DATA_VALIDATIONS2)->f);
+                                    }
                                 }
                             }
 
@@ -1278,15 +1282,14 @@ protected function loadSpreadsheetFromFile(string $filename): Spreadsheet
 
                                         if ($xmlDrawingChildren->oneCellAnchor) {
                                             foreach ($xmlDrawingChildren->oneCellAnchor as $oneCellAnchor) {
+                                                $oneCellAnchor = self::testSimpleXml($oneCellAnchor);
                                                 if ($oneCellAnchor->pic->blipFill) {
                                                     /** @var SimpleXMLElement $blip */
                                                     $blip = $oneCellAnchor->pic->blipFill->children(Namespaces::DRAWINGML)->blip;
                                                     /** @var SimpleXMLElement $xfrm */
                                                     $xfrm = $oneCellAnchor->pic->spPr->children(Namespaces::DRAWINGML)->xfrm;
                                                     /** @var SimpleXMLElement $outerShdw */
                                                     $outerShdw = $oneCellAnchor->pic->spPr->children(Namespaces::DRAWINGML)->effectLst->outerShdw;
-                                                    /** @var SimpleXMLElement $hlinkClick */
-                                                    $hlinkClick = $oneCellAnchor->pic->nvPicPr->cNvPr->children(Namespaces::DRAWINGML)->hlinkClick;
 
                                                     $objDrawing = new \PhpOffice\PhpSpreadsheet\Worksheet\Drawing();
                                                     $objDrawing->setName((string) self::getArrayItem(self::getAttributes($oneCellAnchor->pic->nvPicPr->cNvPr), 'name'));
@@ -1363,11 +1366,11 @@ protected function loadSpreadsheetFromFile(string $filename): Spreadsheet
                                         }
                                         if ($xmlDrawingChildren->twoCellAnchor) {
                                             foreach ($xmlDrawingChildren->twoCellAnchor as $twoCellAnchor) {
+                                                $twoCellAnchor = self::testSimpleXml($twoCellAnchor);
                                                 if ($twoCellAnchor->pic->blipFill) {
                                                     $blip = $twoCellAnchor->pic->blipFill->children(Namespaces::DRAWINGML)->blip;
                                                     $xfrm = $twoCellAnchor->pic->spPr->children(Namespaces::DRAWINGML)->xfrm;
                                                     $outerShdw = $twoCellAnchor->pic->spPr->children(Namespaces::DRAWINGML)->effectLst->outerShdw;
-                                                    $hlinkClick = $twoCellAnchor->pic->nvPicPr->cNvPr->children(Namespaces::DRAWINGML)->hlinkClick;
                                                     $objDrawing = new \PhpOffice\PhpSpreadsheet\Worksheet\Drawing();
                                                     /** @scrutinizer ignore-call */
                                                     $editAs = $twoCellAnchor->attributes();

src/PhpSpreadsheet/Reader/Xlsx/Namespaces.php

@@ -58,6 +58,10 @@ class Namespaces
 
     const VBA = 'http://schemas.microsoft.com/office/2006/relationships/vbaProject';
 
+    const DATA_VALIDATIONS1 = 'http://schemas.microsoft.com/office/spreadsheetml/2009/9/main';
+
+    const DATA_VALIDATIONS2 = 'http://schemas.microsoft.com/office/excel/2006/main';
+
     const DC_ELEMENTS = 'http://purl.org/dc/elements/1.1/';
 
     const DC_TERMS = 'http://purl.org/dc/terms';

tests/PhpSpreadsheetTests/Reader/Xlsx/DataValidationTest.php

@@ -0,0 +1,58 @@
+<?php
+
+namespace PhpOffice\PhpSpreadsheetTests\Reader\Xlsx;
+
+use PhpOffice\PhpSpreadsheet\Cell\DataValidation;
+use PhpOffice\PhpSpreadsheet\Reader\Xlsx;
+use PHPUnit\Framework\TestCase;
+
+class DataValidationTest extends TestCase
+{
+    public function testLoadXlsxDataValidation(): void
+    {
+        $filename = 'tests/data/Reader/XLSX/dataValidationTest.xlsx';
+        $reader = new Xlsx();
+        $spreadsheet = $reader->load($filename);
+
+        $worksheet = $spreadsheet->getActiveSheet();
+
+        self::assertTrue($worksheet->getCell('B3')->hasDataValidation());
+        $spreadsheet->disconnectWorksheets();
+    }
+
+    /**
+     * Test for load drop down lists of another sheet.
+     * Pull #2150, issue #2149. Also issue #2677.
+     *
+     * @dataProvider providerExternalSheet
+     */
+    public function testDataValidationOfAnotherSheet(string $expectedB14, string $filename): void
+    {
+        $reader = new Xlsx();
+        $spreadsheet = $reader->load($filename);
+
+        $worksheet = $spreadsheet->getActiveSheet();
+
+        // same sheet
+        $validationCell = $worksheet->getCell('B5');
+        self::assertTrue($validationCell->hasDataValidation());
+        self::assertSame(DataValidation::TYPE_LIST, $validationCell->getDataValidation()->getType());
+        self::assertSame('$A$5:$A$7', $validationCell->getDataValidation()->getFormula1());
+
+        // another sheet
+        $validationCell = $worksheet->getCell('B14');
+        self::assertTrue($validationCell->hasDataValidation());
+        self::assertSame(DataValidation::TYPE_LIST, $validationCell->getDataValidation()->getType());
+        self::assertSame($expectedB14, $validationCell->getDataValidation()->getFormula1());
+        $spreadsheet->disconnectWorksheets();
+    }
+
+    public function providerExternalSheet(): array
+    {
+        return [
+            'standard spreadsheet' => ['Feuil2!$A$3:$A$5', 'tests/data/Reader/XLSX/dataValidation2Test.xlsx'],
+            'alternate namespace prefix' => ['Feuil2!$A$3:$A$5', 'tests/data/Reader/XLSX/issue.2677.namespace.xlsx'],
+            'missing formula' => ['', 'tests/data/Reader/XLSX/issue.2677.removeformula1.xlsx'],
+        ];
+    }
+}

tests/PhpSpreadsheetTests/Reader/Xlsx/XlsxTest.php

@@ -3,7 +3,6 @@
 namespace PhpOffice\PhpSpreadsheetTests\Reader\Xlsx;
 
 use PhpOffice\PhpSpreadsheet\Cell\Coordinate;
-use PhpOffice\PhpSpreadsheet\Cell\DataValidation;
 use PhpOffice\PhpSpreadsheet\IOFactory;
 use PhpOffice\PhpSpreadsheet\Reader\Xlsx;
 use PhpOffice\PhpSpreadsheet\Shared\File;
@@ -163,42 +162,6 @@ public function testLoadXlsxConditionalFormatting(): void
         self::assertInstanceOf(Style::class, $conditionalRule->getStyle());
     }
 
-    public function testLoadXlsxDataValidation(): void
-    {
-        $filename = 'tests/data/Reader/XLSX/dataValidationTest.xlsx';
-        $reader = new Xlsx();
-        $spreadsheet = $reader->load($filename);
-
-        $worksheet = $spreadsheet->getActiveSheet();
-
-        self::assertTrue($worksheet->getCell('B3')->hasDataValidation());
-    }
-
-    /*
-     * Test for load drop down lists of another sheet.
-     * Pull #2150, issue #2149
-     */
-    public function testLoadXlsxDataValidationOfAnotherSheet(): void
-    {
-        $filename = 'tests/data/Reader/XLSX/dataValidation2Test.xlsx';
-        $reader = new Xlsx();
-        $spreadsheet = $reader->load($filename);
-
-        $worksheet = $spreadsheet->getActiveSheet();
-
-        // same sheet
-        $validationCell = $worksheet->getCell('B5');
-        self::assertTrue($validationCell->hasDataValidation());
-        self::assertSame(DataValidation::TYPE_LIST, $validationCell->getDataValidation()->getType());
-        self::assertSame('$A$5:$A$7', $validationCell->getDataValidation()->getFormula1());
-
-        // another sheet
-        $validationCell = $worksheet->getCell('B14');
-        self::assertTrue($validationCell->hasDataValidation());
-        self::assertSame(DataValidation::TYPE_LIST, $validationCell->getDataValidation()->getType());
-        self::assertSame('Feuil2!$A$3:$A$5', $validationCell->getDataValidation()->getFormula1());
-    }
-
     /**
      * Test load Xlsx file without cell reference.
      *