From 45941c67dd0505dab2fb970786d93055b7cbd2b6 Mon Sep 17 00:00:00 2001 From: Oliver Klee Date: Sun, 15 Oct 2023 20:01:28 +0200 Subject: [PATCH 1/2] Make the type annotation for CompositeExpression::count more specific (#6188) This helps static analysis. | Q | A |------------- | ----------- | Type | improvement | Fixed issues | (none) #### Summary `count` cannot return a negative number. Add a PHPDoc annotation to make this clear for static analysis. --- src/Query/Expression/CompositeExpression.php | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Query/Expression/CompositeExpression.php b/src/Query/Expression/CompositeExpression.php index de1d929a0d0..4cad8ec1d5a 100644 --- a/src/Query/Expression/CompositeExpression.php +++ b/src/Query/Expression/CompositeExpression.php @@ -149,6 +149,7 @@ public function with($part, ...$parts): self * Retrieves the amount of expressions on composite expression. * * @return int + * @psalm-return int<0, max> */ #[ReturnTypeWillChange] public function count() From 4134b86cb986f6fc68fe9ba7c8a7416debfa22bd Mon Sep 17 00:00:00 2001 From: cgknx <124388865+cgknx@users.noreply.github.com> Date: Thu, 2 Nov 2023 20:05:17 +0000 Subject: [PATCH 2/2] MySQLSchemaManager. Check expected database type for json columns only. (#6189) | Q | A |------------- | ----------- | Type | bug fix | Fixed issues | #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. --- src/Schema/MySQLSchemaManager.php | 2 +- .../Schema/CustomIntrospectionTest.php | 80 +++++++++++++++++++ tests/Functional/Schema/Types/Money.php | 18 +++++ tests/Functional/Schema/Types/MoneyType.php | 72 +++++++++++++++++ 4 files changed, 171 insertions(+), 1 deletion(-) create mode 100644 tests/Functional/Schema/CustomIntrospectionTest.php create mode 100644 tests/Functional/Schema/Types/Money.php create mode 100644 tests/Functional/Schema/Types/MoneyType.php diff --git a/src/Schema/MySQLSchemaManager.php b/src/Schema/MySQLSchemaManager.php index 6605aa0bfb8..398eaf01c22 100644 --- a/src/Schema/MySQLSchemaManager.php +++ b/src/Schema/MySQLSchemaManager.php @@ -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); } diff --git a/tests/Functional/Schema/CustomIntrospectionTest.php b/tests/Functional/Schema/CustomIntrospectionTest.php new file mode 100644 index 00000000000..4b7b0a3fb76 --- /dev/null +++ b/tests/Functional/Schema/CustomIntrospectionTest.php @@ -0,0 +1,80 @@ +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), + )); + } +} diff --git a/tests/Functional/Schema/Types/Money.php b/tests/Functional/Schema/Types/Money.php new file mode 100644 index 00000000000..85bae1b2237 --- /dev/null +++ b/tests/Functional/Schema/Types/Money.php @@ -0,0 +1,18 @@ +value = $value; + } + + public function __toString(): string + { + return $this->value; + } +} diff --git a/tests/Functional/Schema/Types/MoneyType.php b/tests/Functional/Schema/Types/MoneyType.php new file mode 100644 index 00000000000..fbfff7f9f54 --- /dev/null +++ b/tests/Functional/Schema/Types/MoneyType.php @@ -0,0 +1,72 @@ +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; + } +}