Skip to content

Commit

Permalink
MySQLSchemaManager. Check expected database type for json columns onl…
Browse files Browse the repository at this point in the history
…y. (doctrine#6189)

|      Q       |   A
|------------- | -----------
| Type         | bug fix
| Fixed issues | doctrine#6185

#### Summary

Using the type from DC2Type comments masks the need to upgrade LONGTEXT
columns to JSON so the underlying column type was also checked for
commented columns.

Custom column types using a valid synonym for column type (e.g. NUMERIC
for DECIMAL), trigger a false failure of this type test.

Restrict the check to json types only and add test case.
  • Loading branch information
cgknx authored and Allan SIMON committed Nov 13, 2023
1 parent 2da3954 commit 2bf4501
Show file tree
Hide file tree
Showing 4 changed files with 171 additions and 1 deletion.
2 changes: 1 addition & 1 deletion src/Schema/MySQLSchemaManager.php
Original file line number Diff line number Diff line change
Expand Up @@ -292,7 +292,7 @@ protected function _getPortableTableColumnDefinition($tableColumn)

// Check underlying database type where doctrine type is inferred from DC2Type comment
// and set a flag if it is not as expected.
if ($origType !== $type && $this->expectedDbType($type, $options) !== $dbType) {
if ($type === 'json' && $origType !== $type && $this->expectedDbType($type, $options) !== $dbType) {
$column->setPlatformOption('declarationMismatch', true);
}

Expand Down
80 changes: 80 additions & 0 deletions tests/Functional/Schema/CustomIntrospectionTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
<?php

declare(strict_types=1);

namespace Doctrine\DBAL\Tests\Functional\Schema;

use Doctrine\DBAL\Platforms\AbstractMySQLPlatform;
use Doctrine\DBAL\Platforms\AbstractPlatform;
use Doctrine\DBAL\Schema\AbstractSchemaManager;
use Doctrine\DBAL\Schema\Comparator;
use Doctrine\DBAL\Schema\Table;
use Doctrine\DBAL\Tests\Functional\Schema\Types\MoneyType;
use Doctrine\DBAL\Tests\FunctionalTestCase;
use Doctrine\DBAL\Types\Type;

use function array_map;
use function implode;
use function sprintf;

/**
* Tests introspection of a custom column type with an underlying decimal column
* on MySQL and MariaDb platforms.
*
* See bug #6185
*/
class CustomIntrospectionTest extends FunctionalTestCase
{
private AbstractSchemaManager $schemaManager;

private Comparator $comparator;

private AbstractPlatform $platform;

public static function setUpBeforeClass(): void
{
Type::addType('money', MoneyType::class);
}

protected function setUp(): void
{
$this->platform = $this->connection->getDatabasePlatform();

if (! $this->platform instanceof AbstractMySQLPlatform) {
self::markTestSkipped();
}

$this->schemaManager = $this->connection->createSchemaManager();
$this->comparator = $this->schemaManager->createComparator();
}

public function testCustomColumnIntrospection(): void
{
$tableName = 'test_custom_column_introspection';
$table = new Table($tableName);

$table->addColumn('id', 'integer');
$table->addColumn('quantity', 'decimal');
$table->addColumn('amount', 'money', [
'notnull' => false,
'scale' => 2,
'precision' => 10,
]);

$this->dropAndCreateTable($table);

$onlineTable = $this->schemaManager->introspectTable($tableName);

$diff = $this->comparator->compareTables($table, $onlineTable);
$changedCols = [];

if (! $diff->isEmpty()) {
$changedCols = array_map(static fn ($c) => $c->getOldColumnName()->getName(), $diff->getModifiedColumns());
}

self::assertTrue($diff->isEmpty(), sprintf(
'Tables should be identical. Differences detected in %s.',
implode(':', $changedCols),
));
}
}
18 changes: 18 additions & 0 deletions tests/Functional/Schema/Types/Money.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
<?php

namespace Doctrine\DBAL\Tests\Functional\Schema\Types;

final class Money
{
private string $value;

public function __construct(string $value)
{
$this->value = $value;
}

public function __toString(): string
{
return $this->value;
}
}
72 changes: 72 additions & 0 deletions tests/Functional/Schema/Types/MoneyType.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
<?php

namespace Doctrine\DBAL\Tests\Functional\Schema\Types;

use Doctrine\DBAL\Platforms\AbstractPlatform;
use Doctrine\DBAL\Types\ConversionException;
use Doctrine\DBAL\Types\Type;
use InvalidArgumentException;

use function is_string;

class MoneyType extends Type
{
public const NAME = 'money';

/**
* {@inheritDoc}
*/
public function getName()
{
return self::NAME;
}

/**
* {@inheritDoc}
*/
public function getSQLDeclaration(array $column, AbstractPlatform $platform): string
{
return $platform->getDecimalTypeDeclarationSQL($column);
}

/**
* {@inheritDoc}
*/
public function convertToDatabaseValue($value, AbstractPlatform $platform): ?string
{
if ($value === null) {
return $value;
}

if ($value instanceof Money) {
return $value->__toString();
}

throw ConversionException::conversionFailedInvalidType($value, $this->getName(), ['null', Money::class]);
}

/**
* {@inheritDoc}
*/
public function convertToPHPValue($value, AbstractPlatform $platform): ?Money
{
if ($value === null || $value instanceof Money) {
return $value;
}

if (! is_string($value)) {
throw ConversionException::conversionFailedInvalidType($value, $this->getName(), ['null', 'string']);
}

try {
return new Money($value);
} catch (InvalidArgumentException $e) {
throw ConversionException::conversionFailedFormat($value, $this->getName(), Money::class, $e);
}
}

public function requiresSQLCommentHint(AbstractPlatform $platform): bool
{
return true;
}
}

0 comments on commit 2bf4501

Please sign in to comment.