Merge branch '2.13.x' into 3.0.x

* 2.13.x: Remove dynamic property declarations (doctrine#9683) PHPStan 1.6.0 (doctrine#9682) Validate XML mapping against XSD file Document ORM drivers only really load ORM metadata Fix HydrationException::invalidDiscriminatorValue parameter type (doctrine#9676) Fix type on ClassMetadata discriminatorMap (doctrine#9675)
derrabus · Apr 26, 2022 · 8fcd93d · 8fcd93d
2 parents aebd3f3 + 5fbe5eb
commit 8fcd93d
Show file tree

Hide file tree

Showing 35 changed files with 378 additions and 373 deletions.
diff --git a/composer.json b/composer.json
@@ -39,7 +39,7 @@
         "doctrine/annotations": "^1.13",
         "doctrine/coding-standard": "^9.0",
         "phpbench/phpbench": "^1.0",
-        "phpstan/phpstan": "1.5.0",
+        "phpstan/phpstan": "1.6.0",
         "phpunit/phpunit": "^9.5",
         "psr/log": "^1 || ^2 || ^3",
         "squizlabs/php_codesniffer": "3.6.2",
@@ -50,6 +50,7 @@
         "doctrine/annotations": "<1.13 || >= 2.0"
     },
     "suggest": {
+        "ext-dom": "Provides support for XSD validation for XML mapping files",
         "symfony/cache": "Provides cache support for Setup Tool with doctrine/cache 2.0"
     },
     "autoload": {

diff --git a/lib/Doctrine/ORM/Internal/Hydration/HydrationException.php b/lib/Doctrine/ORM/Internal/Hydration/HydrationException.php
@@ -56,15 +56,15 @@ public static function missingDiscriminatorMetaMappingColumn(string $entityName,
     }
 
     /**
-     * @param string[] $discrMap
-     * @psalm-param array<string, string> $discrMap
+     * @param string[] $discrValues
+     * @psalm-param list<string> $discrValues
      */
-    public static function invalidDiscriminatorValue(string $discrValue, array $discrMap): self
+    public static function invalidDiscriminatorValue(string $discrValue, array $discrValues): self
     {
         return new self(sprintf(
             'The discriminator value "%s" is invalid. It must be one of "%s".',
             $discrValue,
-            implode('", "', $discrMap)
+            implode('", "', $discrValues)
         ));
     }
 }
diff --git a/lib/Doctrine/ORM/Mapping/ClassMetadataInfo.php b/lib/Doctrine/ORM/Mapping/ClassMetadataInfo.php
@@ -478,7 +478,9 @@ class ClassMetadataInfo implements ClassMetadata
      *
      * @see discriminatorColumn
      *
-     * @var mixed
+     * @var array<string, string>
+     *
+     * @psalm-var array<string, class-string>
      */
     public $discriminatorMap = [];
 

diff --git a/lib/Doctrine/ORM/Mapping/Driver/AnnotationDriver.php b/lib/Doctrine/ORM/Mapping/Driver/AnnotationDriver.php
@@ -10,9 +10,9 @@
 use Doctrine\ORM\Events;
 use Doctrine\ORM\Mapping;
 use Doctrine\ORM\Mapping\Builder\EntityListenerBuilder;
-use Doctrine\ORM\Mapping\ClassMetadataInfo;
+use Doctrine\ORM\Mapping\ClassMetadata;
 use Doctrine\ORM\Mapping\MappingException;
-use Doctrine\Persistence\Mapping\ClassMetadata;
+use Doctrine\Persistence\Mapping\ClassMetadata as PersistenceClassMetadata;
 use Doctrine\Persistence\Mapping\Driver\ColocatedMappingDriver;
 use Doctrine\Persistence\Mapping\Driver\MappingDriver;
 use ReflectionClass;
@@ -68,10 +68,14 @@ public function __construct($reader, $paths = null)
 
     /**
      * {@inheritDoc}
+     *
+     * @psalm-param class-string<T> $className
+     * @psalm-param ClassMetadata<T> $metadata
+     *
+     * @template T of object
      */
-    public function loadMetadataForClass($className, ClassMetadata $metadata)
+    public function loadMetadataForClass($className, PersistenceClassMetadata $metadata)
     {
-        assert($metadata instanceof Mapping\ClassMetadata);
         $class = $metadata->getReflectionClass()
             // this happens when running annotation driver in combination with
             // static reflection services. This is not the nicest fix
@@ -259,7 +263,7 @@ public function loadMetadataForClass($className, ClassMetadata $metadata)
                 constant('Doctrine\ORM\Mapping\ClassMetadata::INHERITANCE_TYPE_' . $inheritanceTypeAnnot->value)
             );
 
-            if ($metadata->inheritanceType !== ClassMetadataInfo::INHERITANCE_TYPE_NONE) {
+            if ($metadata->inheritanceType !== ClassMetadata::INHERITANCE_TYPE_NONE) {
                 // Evaluate DiscriminatorColumn annotation
                 if (isset($classAnnotations[Mapping\DiscriminatorColumn::class])) {
                     $discrColumnAnnot = $classAnnotations[Mapping\DiscriminatorColumn::class];
@@ -504,7 +508,7 @@ public function loadMetadataForClass($className, ClassMetadata $metadata)
     private function loadRelationShipMapping(
         ReflectionProperty $property,
         array &$mapping,
-        ClassMetadata $metadata,
+        PersistenceClassMetadata $metadata,
         array $joinColumns,
         string $className
     ): void {
@@ -608,7 +612,7 @@ private function loadRelationShipMapping(
     /**
      * Attempts to resolve the fetch mode.
      *
-     * @psalm-return \Doctrine\ORM\Mapping\ClassMetadata::FETCH_* The fetch mode as defined in ClassMetadata.
+     * @psalm-return ClassMetadata::FETCH_* The fetch mode as defined in ClassMetadata.
      *
      * @throws MappingException If the fetch mode is not valid.
      */
@@ -624,7 +628,7 @@ private function getFetchMode(string $className, string $fetchMode): int
     /**
      * Attempts to resolve the generated mode.
      *
-     * @psalm-return ClassMetadataInfo::GENERATED_*
+     * @psalm-return ClassMetadata::GENERATED_*
      *
      * @throws MappingException If the fetch mode is not valid.
      */
@@ -724,7 +728,7 @@ private function joinColumnToArray(Mapping\JoinColumn $joinColumn): array
      *                   precision: int,
      *                   notInsertable?: bool,
      *                   notUpdateble?: bool,
-     *                   generated?: ClassMetadataInfo::GENERATED_*,
+     *                   generated?: ClassMetadata::GENERATED_*,
      *                   enumType?: class-string,
      *                   options?: mixed[],
      *                   columnName?: string,

diff --git a/lib/Doctrine/ORM/Mapping/Driver/AttributeDriver.php b/lib/Doctrine/ORM/Mapping/Driver/AttributeDriver.php
@@ -8,9 +8,9 @@
 use Doctrine\ORM\Events;
 use Doctrine\ORM\Mapping;
 use Doctrine\ORM\Mapping\Builder\EntityListenerBuilder;
-use Doctrine\ORM\Mapping\ClassMetadataInfo;
+use Doctrine\ORM\Mapping\ClassMetadata;
 use Doctrine\ORM\Mapping\MappingException;
-use Doctrine\Persistence\Mapping\ClassMetadata;
+use Doctrine\Persistence\Mapping\ClassMetadata as PersistenceClassMetadata;
 use Doctrine\Persistence\Mapping\Driver\ColocatedMappingDriver;
 use Doctrine\Persistence\Mapping\Driver\MappingDriver;
 use ReflectionClass;
@@ -88,10 +88,16 @@ public function isTransient($className)
         return true;
     }
 
-    public function loadMetadataForClass($className, ClassMetadata $metadata): void
+    /**
+     * {@inheritDoc}
+     *
+     * @psalm-param class-string<T> $className
+     * @psalm-param ClassMetadata<T> $metadata
+     *
+     * @template T of object
+     */
+    public function loadMetadataForClass($className, PersistenceClassMetadata $metadata): void
     {
-        assert($metadata instanceof ClassMetadataInfo);
-
         $reflectionClass = $metadata->getReflectionClass();
 
         $classAttributes = $this->reader->getClassAnnotations($reflectionClass);
@@ -228,7 +234,7 @@ public function loadMetadataForClass($className, ClassMetadata $metadata): void
                 constant('Doctrine\ORM\Mapping\ClassMetadata::INHERITANCE_TYPE_' . $inheritanceTypeAttribute->value)
             );
 
-            if ($metadata->inheritanceType !== Mapping\ClassMetadata::INHERITANCE_TYPE_NONE) {
+            if ($metadata->inheritanceType !== ClassMetadata::INHERITANCE_TYPE_NONE) {
                 // Evaluate DiscriminatorColumn annotation
                 if (isset($classAttributes[Mapping\DiscriminatorColumn::class])) {
                     $discrColumnAttribute = $classAttributes[Mapping\DiscriminatorColumn::class];
@@ -481,7 +487,7 @@ public function loadMetadataForClass($className, ClassMetadata $metadata): void
 
                 // Check for `fetch`
                 if ($associationOverride->fetch) {
-                    $override['fetch'] = constant(Mapping\ClassMetadata::class . '::FETCH_' . $associationOverride->fetch);
+                    $override['fetch'] = constant(ClassMetadata::class . '::FETCH_' . $associationOverride->fetch);
                 }
 
                 $metadata->setAssociationOverride($fieldName, $override);
@@ -548,7 +554,7 @@ public function loadMetadataForClass($className, ClassMetadata $metadata): void
      * @param string $className The class name.
      * @param string $fetchMode The fetch mode.
      *
-     * @return int The fetch mode as defined in ClassMetadata.
+     * @return ClassMetadata::FETCH_* The fetch mode as defined in ClassMetadata.
      *
      * @throws MappingException If the fetch mode is not valid.
      */

diff --git a/lib/Doctrine/ORM/Mapping/Driver/DatabaseDriver.php b/lib/Doctrine/ORM/Mapping/Driver/DatabaseDriver.php
@@ -12,9 +12,10 @@
 use Doctrine\DBAL\Types\Types;
 use Doctrine\Inflector\Inflector;
 use Doctrine\Inflector\InflectorFactory;
+use Doctrine\ORM\Mapping\ClassMetadata;
 use Doctrine\ORM\Mapping\ClassMetadataInfo;
 use Doctrine\ORM\Mapping\MappingException;
-use Doctrine\Persistence\Mapping\ClassMetadata;
+use Doctrine\Persistence\Mapping\ClassMetadata as PersistenceClassMetadata;
 use Doctrine\Persistence\Mapping\Driver\MappingDriver;
 use InvalidArgumentException;
 
@@ -159,8 +160,13 @@ public function setInflector(Inflector $inflector): void
 
     /**
      * {@inheritDoc}
+     *
+     * @psalm-param class-string<T> $className
+     * @psalm-param ClassMetadata<T> $metadata
+     *
+     * @template T of object
      */
-    public function loadMetadataForClass($className, ClassMetadata $metadata)
+    public function loadMetadataForClass($className, PersistenceClassMetadata $metadata)
     {
         $this->reverseEngineerMappingFromDatabase();
 

diff --git a/lib/Doctrine/ORM/Mapping/Driver/XmlDriver.php b/lib/Doctrine/ORM/Mapping/Driver/XmlDriver.php
@@ -5,11 +5,13 @@
 namespace Doctrine\ORM\Mapping\Driver;
 
 use Doctrine\Common\Collections\Criteria;
+use Doctrine\Deprecations\Deprecation;
 use Doctrine\ORM\Mapping\Builder\EntityListenerBuilder;
-use Doctrine\ORM\Mapping\ClassMetadata as Metadata;
+use Doctrine\ORM\Mapping\ClassMetadata;
 use Doctrine\ORM\Mapping\MappingException;
-use Doctrine\Persistence\Mapping\ClassMetadata;
+use Doctrine\Persistence\Mapping\ClassMetadata as PersistenceClassMetadata;
 use Doctrine\Persistence\Mapping\Driver\FileDriver;
+use DOMDocument;
 use InvalidArgumentException;
 use LogicException;
 use SimpleXMLElement;
@@ -22,6 +24,9 @@
 use function extension_loaded;
 use function file_get_contents;
 use function in_array;
+use function libxml_clear_errors;
+use function libxml_get_errors;
+use function libxml_use_internal_errors;
 use function simplexml_load_string;
 use function sprintf;
 use function str_replace;
@@ -36,10 +41,13 @@ class XmlDriver extends FileDriver
 {
     public const DEFAULT_FILE_EXTENSION = '.dcm.xml';
 
+    /** @var bool */
+    private $isXsdValidationEnabled;
+
     /**
      * {@inheritDoc}
      */
-    public function __construct($locator, $fileExtension = self::DEFAULT_FILE_EXTENSION)
+    public function __construct($locator, $fileExtension = self::DEFAULT_FILE_EXTENSION, bool $isXsdValidationEnabled = false)
     {
         if (! extension_loaded('simplexml')) {
             throw new LogicException(sprintf(
@@ -48,13 +56,37 @@ public function __construct($locator, $fileExtension = self::DEFAULT_FILE_EXTENS
             ));
         }
 
+        if (! $isXsdValidationEnabled) {
+            Deprecation::trigger(
+                'doctrine/orm',
+                'https://github.com/doctrine/orm/pull/6728',
+                sprintf(
+                    'Using XML mapping driver with XSD validation disabled is deprecated'
+                    . ' and will not be supported in Doctrine ORM 3.0.'
+                )
+            );
+        }
+
+        if ($isXsdValidationEnabled && ! extension_loaded('dom')) {
+            throw new LogicException(sprintf(
+                'XSD validation cannot be enabled because the DOM extension is missing.'
+            ));
+        }
+
+        $this->isXsdValidationEnabled = $isXsdValidationEnabled;
+
         parent::__construct($locator, $fileExtension);
     }
 
     /**
      * {@inheritDoc}
+     *
+     * @psalm-param class-string<T> $className
+     * @psalm-param ClassMetadata<T> $metadata
+     *
+     * @template T of object
      */
-    public function loadMetadataForClass($className, ClassMetadata $metadata)
+    public function loadMetadataForClass($className, PersistenceClassMetadata $metadata)
     {
         $xmlRoot = $this->getElement($className);
         assert($xmlRoot instanceof SimpleXMLElement);
@@ -142,7 +174,7 @@ public function loadMetadataForClass($className, ClassMetadata $metadata)
             $inheritanceType = (string) $xmlRoot['inheritance-type'];
             $metadata->setInheritanceType(constant('Doctrine\ORM\Mapping\ClassMetadata::INHERITANCE_TYPE_' . $inheritanceType));
 
-            if ($metadata->inheritanceType !== Metadata::INHERITANCE_TYPE_NONE) {
+            if ($metadata->inheritanceType !== ClassMetadata::INHERITANCE_TYPE_NONE) {
                 // Evaluate <discriminator-column...>
                 if (isset($xmlRoot->{'discriminator-column'})) {
                     $discrColumn = $xmlRoot->{'discriminator-column'};
@@ -659,7 +691,7 @@ public function loadMetadataForClass($className, ClassMetadata $metadata)
 
                 // Check for `fetch`
                 if (isset($overrideElement['fetch'])) {
-                    $override['fetch'] = constant(Metadata::class . '::FETCH_' . (string) $overrideElement['fetch']);
+                    $override['fetch'] = constant(ClassMetadata::class . '::FETCH_' . (string) $overrideElement['fetch']);
                 }
 
                 $metadata->setAssociationOverride($fieldName, $override);
@@ -909,6 +941,7 @@ private function getCascadeMappings(SimpleXMLElement $cascadeElement): array
      */
     protected function loadMappingFile($file)
     {
+        $this->validateMapping($file);
         $result = [];
         // Note: we do not use `simplexml_load_file()` because of https://bugs.php.net/bug.php?id=62577
         $xmlElement = simplexml_load_string(file_get_contents($file));
@@ -936,6 +969,27 @@ protected function loadMappingFile($file)
         return $result;
     }
 
+    private function validateMapping(string $file): void
+    {
+        if (! $this->isXsdValidationEnabled) {
+            return;
+        }
+
+        $backedUpErrorSetting = libxml_use_internal_errors(true);
+
+        try {
+            $document = new DOMDocument();
+            $document->load($file);
+
+            if (! $document->schemaValidate(__DIR__ . '/../../../../../doctrine-mapping.xsd')) {
+                throw MappingException::fromLibXmlErrors(libxml_get_errors());
+            }
+        } finally {
+            libxml_clear_errors();
+            libxml_use_internal_errors($backedUpErrorSetting);
+        }
+    }
+
     /**
      * @param mixed $element
      *

diff --git a/lib/Doctrine/ORM/Mapping/MappingException.php b/lib/Doctrine/ORM/Mapping/MappingException.php
@@ -6,6 +6,7 @@
 
 use BackedEnum;
 use Doctrine\ORM\Exception\ORMException;
+use LibXMLError;
 use Exception;
 use ReflectionException;
 use ValueError;
@@ -18,6 +19,8 @@
 use function implode;
 use function sprintf;
 
+use const PHP_EOL;
+
 /**
  * A MappingException indicates that something is wrong with the mapping setup.
  */
@@ -984,4 +987,21 @@ public static function invalidEnumValue(
             $enumType
         ), 0, $previous);
     }
+
+    /**
+     * @param LibXMLError[] $errors
+     */
+    public static function fromLibXmlErrors(array $errors): self
+    {
+        $formatter = static function (LibXMLError $error): string {
+            return sprintf(
+                'libxml error: %s in %s at line %d',
+                $error->message,
+                $error->file,
+                $error->line
+            );
+        };
+
+        return new self(implode(PHP_EOL, array_map($formatter, $errors)));
+    }
 }
diff --git a/lib/Doctrine/ORM/ORMSetup.php b/lib/Doctrine/ORM/ORMSetup.php
@@ -99,10 +99,11 @@ public static function createXMLMetadataConfiguration(
         array $paths,
         bool $isDevMode = false,
         ?string $proxyDir = null,
-        ?CacheItemPoolInterface $cache = null
+        ?CacheItemPoolInterface $cache = null,
+        bool $isXsdValidationEnabled = false
     ): Configuration {
         $config = self::createConfiguration($isDevMode, $proxyDir, $cache);
-        $config->setMetadataDriverImpl(new XmlDriver($paths));
+        $config->setMetadataDriverImpl(new XmlDriver($paths, XmlDriver::DEFAULT_FILE_EXTENSION, $isXsdValidationEnabled));
 
         return $config;
     }