Merge pull request doctrine#11165 from jwage/allow-xml-validation-dis…

…able Allow XML validation to be disabled but keep it enabled by default.
derrabus · Jan 26, 2024 · 1a5942a · 1a5942a
2 parents 69cc78c + 282b8fb
commit 1a5942a
Show file tree

Hide file tree

Showing 8 changed files with 67 additions and 40 deletions.
diff --git a/UPGRADE.md b/UPGRADE.md
@@ -76,12 +76,11 @@ explicitly forbidden to point out mistakes.
 
 You should use `DEFERRED_EXPLICIT` instead.
 
-## BC BREAK: `Mapping\Driver\XmlDriver::__construct()` third argument is now a no-op
+## BC BREAK: `Mapping\Driver\XmlDriver::__construct()` third argument is now enabled by default
 
 The third argument to
 `Doctrine\ORM\Mapping\Driver\XmlDriver::__construct()` was introduced to
-let users opt-in to XML validation, that is now always enabled, regardless of
-the value of that argument.
+let users opt-in to XML validation, that is now always enabled by default.
 
 As a consequence, the same goes for
 `Doctrine\ORM\Mapping\Driver\SimplifiedXmlDriver`, and for

diff --git a/src/Mapping/Driver/SimplifiedXmlDriver.php b/src/Mapping/Driver/SimplifiedXmlDriver.php
@@ -5,9 +5,6 @@
 namespace Doctrine\ORM\Mapping\Driver;
 
 use Doctrine\Persistence\Mapping\Driver\SymfonyFileLocator;
-use InvalidArgumentException;
-
-use function sprintf;
 
 /**
  * XmlDriver that additionally looks for mapping information in a global file.
@@ -21,15 +18,8 @@ class SimplifiedXmlDriver extends XmlDriver
      */
     public function __construct($prefixes, $fileExtension = self::DEFAULT_FILE_EXTENSION, bool $isXsdValidationEnabled = true)
     {
-        if (! $isXsdValidationEnabled) {
-            throw new InvalidArgumentException(sprintf(
-                'The $isXsdValidationEnabled argument is no longer supported, make sure to omit it when calling %s.',
-                __METHOD__,
-            ));
-        }
-
         $locator = new SymfonyFileLocator((array) $prefixes, $fileExtension);
 
-        parent::__construct($locator, $fileExtension);
+        parent::__construct($locator, $fileExtension, $isXsdValidationEnabled);
     }
 }
diff --git a/src/Mapping/Driver/XmlDriver.php b/src/Mapping/Driver/XmlDriver.php
@@ -43,13 +43,11 @@ class XmlDriver extends FileDriver
 
     /**
      * {@inheritDoc}
-     *
-     * @param true $isXsdValidationEnabled no-op, will be removed in 4.0
      */
     public function __construct(
         string|array|FileLocator $locator,
         string $fileExtension = self::DEFAULT_FILE_EXTENSION,
-        bool $isXsdValidationEnabled = true,
+        private readonly bool $isXsdValidationEnabled = true,
     ) {
         if (! extension_loaded('simplexml')) {
             throw new LogicException(
@@ -58,14 +56,7 @@ public function __construct(
             );
         }
 
-        if (! $isXsdValidationEnabled) {
-            throw new InvalidArgumentException(sprintf(
-                'The $isXsdValidationEnabled argument is no longer supported, make sure to omit it when calling %s.',
-                __METHOD__,
-            ));
-        }
-
-        if (! extension_loaded('dom')) {
+        if ($isXsdValidationEnabled && ! extension_loaded('dom')) {
             throw new LogicException(
                 'XSD validation cannot be enabled because the DOM extension is missing.',
             );
@@ -914,6 +905,10 @@ protected function loadMappingFile($file)
 
     private function validateMapping(string $file): void
     {
+        if (! $this->isXsdValidationEnabled) {
+            return;
+        }
+
         $backedUpErrorSetting = libxml_use_internal_errors(true);
 
         try {

diff --git a/src/ORMSetup.php b/src/ORMSetup.php
@@ -6,7 +6,6 @@
 
 use Doctrine\ORM\Mapping\Driver\AttributeDriver;
 use Doctrine\ORM\Mapping\Driver\XmlDriver;
-use InvalidArgumentException;
 use Psr\Cache\CacheItemPoolInterface;
 use Redis;
 use RuntimeException;
@@ -19,7 +18,6 @@
 use function class_exists;
 use function extension_loaded;
 use function md5;
-use function sprintf;
 use function sys_get_temp_dir;
 
 final class ORMSetup
@@ -54,13 +52,6 @@ public static function createXMLMetadataConfiguration(
         CacheItemPoolInterface|null $cache = null,
         bool $isXsdValidationEnabled = true,
     ): Configuration {
-        if (! $isXsdValidationEnabled) {
-            throw new InvalidArgumentException(sprintf(
-                'The $isXsdValidationEnabled argument is no longer supported, make sure to omit it when calling %s.',
-                __METHOD__,
-            ));
-        }
-
         $config = self::createConfiguration($isDevMode, $proxyDir, $cache);
         $config->setMetadataDriverImpl(new XmlDriver($paths, XmlDriver::DEFAULT_FILE_EXTENSION, $isXsdValidationEnabled));
 

diff --git a/tests/Tests/Models/InvalidXml.php b/tests/Tests/Models/InvalidXml.php
@@ -0,0 +1,9 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Doctrine\Tests\Models;
+
+class InvalidXml
+{
+}
diff --git a/tests/Tests/ORM/Mapping/XmlMappingDriverTest.php b/tests/Tests/ORM/Mapping/XmlMappingDriverTest.php
@@ -23,13 +23,13 @@
 use Doctrine\Tests\Models\Generic\BooleanModel;
 use Doctrine\Tests\Models\GH7141\GH7141Article;
 use Doctrine\Tests\Models\GH7316\GH7316Article;
+use Doctrine\Tests\Models\InvalidXml;
 use Doctrine\Tests\Models\Project\Project;
 use Doctrine\Tests\Models\Project\ProjectId;
 use Doctrine\Tests\Models\Project\ProjectInvalidMapping;
 use Doctrine\Tests\Models\Project\ProjectName;
 use Doctrine\Tests\Models\ValueObjects\Name;
 use Doctrine\Tests\Models\ValueObjects\Person;
-use InvalidArgumentException;
 use PHPUnit\Framework\Attributes\DataProvider;
 use PHPUnit\Framework\Attributes\Group;
 
@@ -301,16 +301,49 @@ public function testClassNameInFieldOrId(): void
         self::assertEquals(ProjectName::class, $name['type']);
     }
 
-    public function testDisablingXmlValidationIsNotPossible(): void
+    public function testDisablingXmlValidationIsPossible(): void
     {
-        $this->expectException(InvalidArgumentException::class);
+        $this->expectNotToPerformAssertions();
 
         new XmlDriver(
             __DIR__ . DIRECTORY_SEPARATOR . 'xml',
             XmlDriver::DEFAULT_FILE_EXTENSION,
             false,
         );
     }
+
+    public function testXmlValidationEnabled(): void
+    {
+        $driver = new XmlDriver(
+            __DIR__ . DIRECTORY_SEPARATOR . 'invalid_xml',
+            XmlDriver::DEFAULT_FILE_EXTENSION,
+            true,
+        );
+
+        $class = new ClassMetadata(InvalidXml::class);
+        $class->initializeReflection(new RuntimeReflectionService());
+
+        self::expectException(MappingException::class);
+        self::expectExceptionMessage("libxml error: Element '{http://doctrine-project.org/schemas/orm/doctrine-mapping}field', attribute 'invalid': The attribute 'invalid' is not allowed.");
+
+        $driver->loadMetadataForClass(InvalidXml::class, $class);
+    }
+
+    public function testXmlValidationDisabled(): void
+    {
+        $driver = new XmlDriver(
+            __DIR__ . DIRECTORY_SEPARATOR . 'invalid_xml',
+            XmlDriver::DEFAULT_FILE_EXTENSION,
+            false,
+        );
+
+        $class = new ClassMetadata(InvalidXml::class);
+        $class->initializeReflection(new RuntimeReflectionService());
+
+        $driver->loadMetadataForClass(InvalidXml::class, $class);
+
+        self::assertCount(1, $class->fieldMappings);
+    }
 }
 
 class CTI

diff --git a/tests/Tests/ORM/Mapping/invalid_xml/Doctrine.Tests.Models.InvalidXml.dcm.xml b/tests/Tests/ORM/Mapping/invalid_xml/Doctrine.Tests.Models.InvalidXml.dcm.xml
@@ -0,0 +1,11 @@
+<?xml version="1.0" encoding="UTF-8"?>
+
+<doctrine-mapping xmlns="http://doctrine-project.org/schemas/orm/doctrine-mapping"
+      xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+      xsi:schemaLocation="http://doctrine-project.org/schemas/orm/doctrine-mapping
+                          https://www.doctrine-project.org/schemas/orm/doctrine-mapping.xsd">
+
+    <entity name="Doctrine\Tests\Models\InvalidXml" table="invalid_xml">
+        <field name="email" type="string" invalid="true" />
+    </entity>
+</doctrine-mapping>
diff --git a/tests/Tests/ORM/ORMSetupTest.php b/tests/Tests/ORM/ORMSetupTest.php
@@ -9,7 +9,6 @@
 use Doctrine\ORM\Mapping\Driver\AttributeDriver;
 use Doctrine\ORM\Mapping\Driver\XmlDriver;
 use Doctrine\ORM\ORMSetup;
-use InvalidArgumentException;
 use PHPUnit\Framework\Attributes\Group;
 use PHPUnit\Framework\Attributes\RequiresPhpExtension;
 use PHPUnit\Framework\Attributes\RequiresSetting;
@@ -41,9 +40,9 @@ public function testXMLConfiguration(): void
         self::assertInstanceOf(XmlDriver::class, $config->getMetadataDriverImpl());
     }
 
-    public function testDisablingXmlValidationIsNotPossible(): void
+    public function testDisablingXmlValidationIsPossible(): void
     {
-        $this->expectException(InvalidArgumentException::class);
+        $this->expectNotToPerformAssertions();
 
         ORMSetup::createXMLMetadataConfiguration(paths: [], isXsdValidationEnabled: false);
     }