Skip to content

Commit

Permalink
Adds metadata field type validation against Entity property type
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
DavideBicego authored and greg0ire committed Sep 15, 2023
1 parent a2d2e17 commit 0f67ba2
Show file tree
Hide file tree
Showing 4 changed files with 205 additions and 0 deletions.
107 changes: 107 additions & 0 deletions lib/Doctrine/ORM/Tools/SchemaValidator.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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";

Expand Down Expand Up @@ -304,4 +349,66 @@ public function getUpdateSchemaList(): array

return $schemaTool->getUpdateSchemaSql($allMetadata, true);
}

/** @return list<string> 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;

Check warning on line 367 in lib/Doctrine/ORM/Tools/SchemaValidator.php

View check run for this annotation

Codecov / codecov/patch

lib/Doctrine/ORM/Tools/SchemaValidator.php#L367

Added line #L367 was not covered by tests
}

// 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;

Check warning on line 379 in lib/Doctrine/ORM/Tools/SchemaValidator.php

View check run for this annotation

Codecov / codecov/patch

lib/Doctrine/ORM/Tools/SchemaValidator.php#L379

Added line #L379 was not covered by tests
}

$propertyType = $propertyType->getName();

// If the property type is the same as the metadata field type, we are ok
if ($propertyType === $metadataFieldType) {
return null;

Check warning on line 386 in lib/Doctrine/ORM/Tools/SchemaValidator.php

View check run for this annotation

Codecov / codecov/patch

lib/Doctrine/ORM/Tools/SchemaValidator.php#L386

Added line #L386 was not covered by tests
}

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;
}
}
53 changes: 53 additions & 0 deletions tests/Doctrine/Tests/ORM/Functional/Ticket/GH10661/GH10661Test.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
<?php

declare(strict_types=1);

namespace Doctrine\Tests\ORM\Functional\Ticket\GH10661;

use Doctrine\ORM\EntityManagerInterface;
use Doctrine\ORM\Tools\SchemaValidator;
use Doctrine\Tests\OrmTestCase;

/**
* @requires PHP >= 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
);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
<?php

declare(strict_types=1);

namespace Doctrine\Tests\ORM\Functional\Ticket\GH10661;

use Doctrine\ORM\Mapping\Column;
use Doctrine\ORM\Mapping\Entity;

/** @Entity */
class InvalidChildEntity extends InvalidEntity
{
/** @Column(type="string") */
protected int $property2;

/**
* @Column(type="boolean")
*/
private string $anotherProperty;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
<?php

declare(strict_types=1);

namespace Doctrine\Tests\ORM\Functional\Ticket\GH10661;

use Doctrine\ORM\Mapping\Column;
use Doctrine\ORM\Mapping\Entity;
use Doctrine\ORM\Mapping\Id;

/** @Entity */
class InvalidEntity
{
/**
* @var int
* @Id
* @Column
*/
protected $key;

/**
* @Column(type="decimal")
*/
protected float $property1;
}

0 comments on commit 0f67ba2

Please sign in to comment.