From 0f67ba2176017181347d1647ce22f93b43403d74 Mon Sep 17 00:00:00 2001 From: DavideBicego Date: Thu, 27 Apr 2023 15:58:52 +0200 Subject: [PATCH] Adds metadata field type validation against Entity property type This avoid situations where you might map a decimal to a float, when it should really be mapped to a string. This is a big pitfall because in that situation, the ORM will consider the field changed every time. --- lib/Doctrine/ORM/Tools/SchemaValidator.php | 107 ++++++++++++++++++ .../Functional/Ticket/GH10661/GH10661Test.php | 53 +++++++++ .../Ticket/GH10661/InvalidChildEntity.php | 20 ++++ .../Ticket/GH10661/InvalidEntity.php | 25 ++++ 4 files changed, 205 insertions(+) create mode 100644 tests/Doctrine/Tests/ORM/Functional/Ticket/GH10661/GH10661Test.php create mode 100644 tests/Doctrine/Tests/ORM/Functional/Ticket/GH10661/InvalidChildEntity.php create mode 100644 tests/Doctrine/Tests/ORM/Functional/Ticket/GH10661/InvalidEntity.php diff --git a/lib/Doctrine/ORM/Tools/SchemaValidator.php b/lib/Doctrine/ORM/Tools/SchemaValidator.php index 28ec5c12066..c6948aa85ff 100644 --- a/lib/Doctrine/ORM/Tools/SchemaValidator.php +++ b/lib/Doctrine/ORM/Tools/SchemaValidator.php @@ -4,33 +4,73 @@ namespace Doctrine\ORM\Tools; +use Doctrine\DBAL\Types\AsciiStringType; +use Doctrine\DBAL\Types\BigIntType; +use Doctrine\DBAL\Types\BooleanType; +use Doctrine\DBAL\Types\DecimalType; +use Doctrine\DBAL\Types\FloatType; +use Doctrine\DBAL\Types\GuidType; +use Doctrine\DBAL\Types\IntegerType; +use Doctrine\DBAL\Types\JsonType; +use Doctrine\DBAL\Types\SimpleArrayType; +use Doctrine\DBAL\Types\SmallIntType; +use Doctrine\DBAL\Types\StringType; +use Doctrine\DBAL\Types\TextType; use Doctrine\DBAL\Types\Type; use Doctrine\Deprecations\Deprecation; use Doctrine\ORM\EntityManagerInterface; use Doctrine\ORM\Mapping\ClassMetadata; use Doctrine\ORM\Mapping\ClassMetadataInfo; +use ReflectionNamedType; use function array_diff; +use function array_filter; use function array_key_exists; +use function array_map; +use function array_push; use function array_search; use function array_values; +use function assert; use function class_exists; use function class_parents; use function count; use function get_class; use function implode; use function in_array; +use function sprintf; + +use const PHP_VERSION_ID; /** * Performs strict validation of the mapping schema * * @link www.doctrine-project.com + * + * @psalm-import-type FieldMapping from ClassMetadata */ class SchemaValidator { /** @var EntityManagerInterface */ private $em; + /** + * It maps built-in Doctrine types to PHP types + */ + private const BUILTIN_TYPES_MAP = [ + AsciiStringType::class => 'string', + BigIntType::class => 'string', + BooleanType::class => 'bool', + DecimalType::class => 'string', + FloatType::class => 'float', + GuidType::class => 'string', + IntegerType::class => 'int', + JsonType::class => 'array', + SimpleArrayType::class => 'array', + SmallIntType::class => 'int', + StringType::class => 'string', + TextType::class => 'string', + ]; + public function __construct(EntityManagerInterface $em) { $this->em = $em; @@ -92,6 +132,11 @@ public function validateClass(ClassMetadataInfo $class) } } + // PHP 7.4 introduces the ability to type properties, so we can't validate them in previous versions + if (PHP_VERSION_ID >= 70400) { + array_push($ce, ...$this->validatePropertiesTypes($class)); + } + if ($class->isEmbeddedClass && count($class->associationMappings) > 0) { $ce[] = "Embeddable '" . $class->name . "' does not support associations"; @@ -304,4 +349,66 @@ public function getUpdateSchemaList(): array return $schemaTool->getUpdateSchemaSql($allMetadata, true); } + + /** @return list containing the found issues */ + private function validatePropertiesTypes(ClassMetadataInfo $class): array + { + return array_values( + array_filter( + array_map( + /** @param FieldMapping $fieldMapping */ + function (array $fieldMapping) use ($class): ?string { + $fieldName = $fieldMapping['fieldName']; + assert(isset($class->reflFields[$fieldName])); + $propertyType = $class->reflFields[$fieldName]->getType(); + + // If the field type is not a built-in type, we cannot check it + if (! Type::hasType($fieldMapping['type'])) { + return null; + } + + // If the property type is not a named type, we cannot check it + if (! ($propertyType instanceof ReflectionNamedType)) { + return null; + } + + $metadataFieldType = $this->findBuiltInType(Type::getType($fieldMapping['type'])); + + //If the metadata field type is not a mapped built-in type, we cannot check it + if ($metadataFieldType === null) { + return null; + } + + $propertyType = $propertyType->getName(); + + // If the property type is the same as the metadata field type, we are ok + if ($propertyType === $metadataFieldType) { + return null; + } + + return sprintf( + "The field '%s#%s' has the property type '%s' that differs from the metadata field type '%s' returned by the '%s' DBAL type.", + $class->name, + $fieldName, + $propertyType, + $metadataFieldType, + $fieldMapping['type'] + ); + }, + $class->fieldMappings + ) + ) + ); + } + + /** + * The exact DBAL type must be used (no subclasses), since consumers of doctrine/orm may have their own + * customization around field types. + */ + private function findBuiltInType(Type $type): ?string + { + $typeName = get_class($type); + + return self::BUILTIN_TYPES_MAP[$typeName] ?? null; + } } diff --git a/tests/Doctrine/Tests/ORM/Functional/Ticket/GH10661/GH10661Test.php b/tests/Doctrine/Tests/ORM/Functional/Ticket/GH10661/GH10661Test.php new file mode 100644 index 00000000000..6bc7f4a6124 --- /dev/null +++ b/tests/Doctrine/Tests/ORM/Functional/Ticket/GH10661/GH10661Test.php @@ -0,0 +1,53 @@ += 7.4 + */ +final class GH10661Test extends OrmTestCase +{ + /** @var EntityManagerInterface */ + private $em; + + /** @var SchemaValidator */ + private $validator; + + protected function setUp(): void + { + $this->em = $this->getTestEntityManager(); + $this->validator = new SchemaValidator($this->em); + } + + public function testMetadataFieldTypeNotCoherentWithEntityPropertyType(): void + { + $class = $this->em->getClassMetadata(InvalidEntity::class); + $ce = $this->validator->validateClass($class); + + self::assertEquals( + ["The field 'Doctrine\Tests\ORM\Functional\Ticket\GH10661\InvalidEntity#property1' has the property type 'float' that differs from the metadata field type 'string' returned by the 'decimal' DBAL type."], + $ce + ); + } + + public function testMetadataFieldTypeNotCoherentWithEntityPropertyTypeWithInheritance(): void + { + $class = $this->em->getClassMetadata(InvalidChildEntity::class); + $ce = $this->validator->validateClass($class); + + self::assertEquals( + [ + "The field 'Doctrine\Tests\ORM\Functional\Ticket\GH10661\InvalidChildEntity#property1' has the property type 'float' that differs from the metadata field type 'string' returned by the 'decimal' DBAL type.", + "The field 'Doctrine\Tests\ORM\Functional\Ticket\GH10661\InvalidChildEntity#property2' has the property type 'int' that differs from the metadata field type 'string' returned by the 'string' DBAL type.", + "The field 'Doctrine\Tests\ORM\Functional\Ticket\GH10661\InvalidChildEntity#anotherProperty' has the property type 'string' that differs from the metadata field type 'bool' returned by the 'boolean' DBAL type.", + ], + $ce + ); + } +} diff --git a/tests/Doctrine/Tests/ORM/Functional/Ticket/GH10661/InvalidChildEntity.php b/tests/Doctrine/Tests/ORM/Functional/Ticket/GH10661/InvalidChildEntity.php new file mode 100644 index 00000000000..123b8b0f436 --- /dev/null +++ b/tests/Doctrine/Tests/ORM/Functional/Ticket/GH10661/InvalidChildEntity.php @@ -0,0 +1,20 @@ +