Fixed passing DateTime instances into DoctrineDatabase::findBy() methods

phpunit.xml.dist

@@ -12,4 +12,10 @@
             <directory>tests/lib</directory>
         </testsuite>
     </testsuites>
+    <php>
+        <env name="SYMFONY_DEPRECATIONS_HELPER" value="baselineFile=./tests/allowed.json" />
+    </php>
+    <listeners>
+        <listener class="Symfony\Bridge\PhpUnit\SymfonyTestsListener"/>
+    </listeners>
 </phpunit>

src/contracts/Gateway/AbstractDoctrineDatabase.php

@@ -11,6 +11,7 @@
 use Doctrine\Common\Collections\Expr\Expression;
 use Doctrine\DBAL\Connection;
 use Doctrine\DBAL\Query\QueryBuilder;
+use Doctrine\DBAL\Types\ConversionException;
 use Ibexa\CorePersistence\Gateway\ExpressionVisitor;
 use InvalidArgumentException;
 
@@ -116,8 +117,6 @@ public function countAll(): int
     }
 
     /**
-     * @param \Doctrine\Common\Collections\Expr\Expression|array<string, \Doctrine\Common\Collections\Expr\Expression|scalar|array<scalar>|null> $criteria
-     *
      * @throws \Doctrine\DBAL\Driver\Exception
      * @throws \Doctrine\DBAL\Exception
      * @throws \Ibexa\Contracts\CorePersistence\Exception\MappingException
@@ -257,7 +256,7 @@ private function applyInheritance(QueryBuilder $qb): void
     }
 
     /**
-     * @param \Doctrine\Common\Collections\Expr\Expression|array<string, \Doctrine\Common\Collections\Expr\Expression|scalar|array<scalar>|null> $criteria
+     * @param \Doctrine\Common\Collections\Expr\Expression|array<string, \Doctrine\Common\Collections\Expr\Expression|scalar|\DateTimeInterface|array<scalar|\DateTimeInterface>|null> $criteria
      *
      * @return \Doctrine\DBAL\Query\Expression\CompositeExpression|string|null
      *
@@ -325,7 +324,7 @@ private function buildConditionExpression(ExpressionVisitor $visitor, QueryBuild
     }
 
     /**
-     * @param scalar|array<scalar>|null $value
+     * @param scalar|\DateTimeInterface|array<scalar|\DateTimeInterface>|null $value
      *
      * @throws \Doctrine\DBAL\Exception
      * @throws \Ibexa\Contracts\CorePersistence\Exception\MappingException
@@ -382,7 +381,21 @@ private function buildCondition(QueryBuilder $qb, string $column, $value): strin
             return $qb->expr()->isNull($fullColumnName);
         }
 
+        $columnType = $metadata->getColumnType($column);
+        $platform = $this->connection->getDatabasePlatform();
+
         if (is_array($value)) {
+            $value = array_map(
+                static function ($value) use ($columnType, $platform) {
+                    try {
+                        return $columnType->convertToDatabaseValue($value, $platform);
+                    } catch (ConversionException $e) {
+                        return $value;
+                    }
+                },
+                $value,
+            );
+
             $parameter = $qb->createPositionalParameter(
                 $value,
                 $columnBinding + Connection::ARRAY_PARAM_OFFSET
@@ -391,6 +404,11 @@ private function buildCondition(QueryBuilder $qb, string $column, $value): strin
             return $qb->expr()->in($fullColumnName, $parameter);
         }
 
+        try {
+            $value = $columnType->convertToDatabaseValue($value, $this->connection->getDatabasePlatform());
+        } catch (ConversionException $e) {
+        }
+
         $parameter = $qb->createPositionalParameter($value, $columnBinding);
 
         return $qb->expr()->eq($fullColumnName, $parameter);
@@ -429,7 +447,7 @@ final protected function applyOrderBy(QueryBuilder $qb, ?array $orderBy = []): v
     }
 
     /**
-     * @param \Doctrine\Common\Collections\Expr\Expression|array<string, \Doctrine\Common\Collections\Expr\Expression|scalar|array<scalar>|null> $criteria
+     * @param \Doctrine\Common\Collections\Expr\Expression|array<string, \Doctrine\Common\Collections\Expr\Expression|scalar|\DateTimeInterface|array<scalar|\DateTimeInterface>|null> $criteria
      */
     final protected function applyCriteria(QueryBuilder $qb, $criteria): void
     {

src/contracts/Gateway/DoctrineSchemaMetadata.php

@@ -236,17 +236,36 @@ public function convertToPHPValues(array $data): array
     /**
      * @throws \Doctrine\DBAL\Exception
      */
-    public function convertToDatabaseValues(array $data): array
+    public function convertToPHPValue(string $columnName, $value)
     {
         $platform = $this->connection->getDatabasePlatform();
+
+        return $this->getColumnType($columnName)->convertToPHPValue($value, $platform);
+    }
+
+    /**
+     * @throws \Doctrine\DBAL\Exception
+     */
+    public function convertToDatabaseValues(array $data): array
+    {
         $result = [];
         foreach ($data as $columnName => $value) {
-            $result[$columnName] = $this->getColumnType($columnName)->convertToDatabaseValue($value, $platform);
+            $result[$columnName] = $this->convertToDatabaseValue($columnName, $value);
         }
 
         return $result;
     }
 
+    /**
+     * @throws \Doctrine\DBAL\Exception
+     */
+    public function convertToDatabaseValue(string $column, $value)
+    {
+        $platform = $this->connection->getDatabasePlatform();
+
+        return $this->getColumnType($column)->convertToDatabaseValue($value, $platform);
+    }
+
     /**
      * @param array<string, mixed> $data
      *

src/contracts/Gateway/DoctrineSchemaMetadataInterface.php

@@ -67,6 +67,13 @@ public function isInheritedColumn(string $column): bool;
      */
     public function convertToPHPValues(array $data): array;
 
+    /**
+     * @param mixed $value
+     *
+     * @return mixed
+     */
+    public function convertToPHPValue(string $columnName, $value);
+
     /**
      * Similarly to Doctrine\DBAL\Types\Type::convertToDatabaseValue, converts PHP representation to database
      * representation.
@@ -81,6 +88,13 @@ public function convertToPHPValues(array $data): array;
      */
     public function convertToDatabaseValues(array $data): array;
 
+    /**
+     * @param mixed $value
+     *
+     * @return mixed
+     */
+    public function convertToDatabaseValue(string $column, $value);
+
     /**
      * @param array<string, mixed> $data
      *

src/contracts/Gateway/GatewayInterface.php

@@ -23,7 +23,7 @@ public function getMetadata(): DoctrineSchemaMetadataInterface;
     public function countAll(): int;
 
     /**
-     * @param \Doctrine\Common\Collections\Expr\Expression|array<\Doctrine\Common\Collections\Expr\Expression|scalar|array<scalar>> $criteria
+     * @param \Doctrine\Common\Collections\Expr\Expression|array<\Doctrine\Common\Collections\Expr\Expression|scalar|\DateTimeInterface|array<scalar|\DateTimeInterface>> $criteria
      */
     public function countBy($criteria): int;
 
@@ -33,7 +33,7 @@ public function countBy($criteria): int;
     public function findAll(?int $limit = null, int $offset = 0): array;
 
     /**
-     * @param \Doctrine\Common\Collections\Expr\Expression|array<\Doctrine\Common\Collections\Expr\Expression|scalar|array<scalar>|null> $criteria Map of column names to values that will be used as part of WHERE query
+     * @param \Doctrine\Common\Collections\Expr\Expression|array<\Doctrine\Common\Collections\Expr\Expression|scalar|\DateTimeInterface|array<scalar|\DateTimeInterface>|null> $criteria Map of column names to values that will be used as part of WHERE query
      * @param array<string, string>|null $orderBy Map of column names to "ASC" or "DESC", that will be used in SORT query
      *
      * @phpstan-param array<string, "ASC"|"DESC">|null $orderBy
@@ -43,7 +43,7 @@ public function findAll(?int $limit = null, int $offset = 0): array;
     public function findBy($criteria, ?array $orderBy = null, ?int $limit = null, int $offset = 0): array;
 
     /**
-     * @param array<string, \Doctrine\Common\Collections\Expr\Expression|scalar|array<scalar>|null> $criteria Map of column names to values that will be used as part of WHERE query
+     * @param array<string, \Doctrine\Common\Collections\Expr\Expression|scalar|\DateTimeInterface|array<scalar|\DateTimeInterface>|null> $criteria Map of column names to values that will be used as part of WHERE query
      * @param array<string, string>|null $orderBy Map of column names to "ASC" or "DESC", that will be used in SORT query
      *
      * @phpstan-param array<string, "ASC"|"DESC">|null $orderBy

src/lib/Gateway/ExpressionVisitor.php

@@ -116,6 +116,8 @@ public function walkComparison(Comparison $comparison)
         } else {
             $fullColumnName = $this->tableAlias . '.' . $column;
         }
+
+        $value = $this->schemaMetadata->convertToDatabaseValue($column, $value);
         $parameter = new Parameter($parameterName, $value, $type);
 
         return $this->handleComparison($comparison, $parameter, $fullColumnName, $placeholder);

tests/allowed.json

@@ -0,0 +1,12 @@
+[
+    {
+        "location": "Doctrine\\DBAL\\Platforms\\PostgreSqlPlatform",
+        "message": "Using ${var} in strings is deprecated, use {$var} instead",
+        "count": 1
+    },
+    {
+        "location": "Ibexa\\Tests\\CorePersistence\\Gateway\\AbstractDoctrineDatabaseTest::testDateTimeQueries",
+        "message": "The Doctrine\\DBAL\\Driver\\ResultStatement::fetch method is deprecated (Use fetchNumeric(), fetchAssociative() or fetchOne() instead.).",
+        "count": 1
+    }
+]

tests/bundle/Gateway/ExpressionVisitorTest.php

@@ -437,5 +437,13 @@ private function configureFieldInMetadata(DoctrineSchemaMetadataInterface $metad
                 ...array_map(static fn (string $fieldName): IsIdentical => self::identicalTo($fieldName), $fields),
             ))
             ->willReturn(true);
+
+        $metadata
+            ->expects(self::atLeastOnce())
+            ->method('convertToDatabaseValue')
+            ->with(self::logicalOr(
+                ...array_map(static fn (string $fieldName): IsIdentical => self::identicalTo($fieldName), $fields),
+            ))
+            ->willReturnArgument(1);
     }
 }

tests/lib/Gateway/AbstractDoctrineDatabaseTest.php

@@ -0,0 +1,176 @@
+<?php
+
+/**
+ * @copyright Copyright (C) Ibexa AS. All rights reserved.
+ * @license For full copyright and license information view LICENSE file distributed with this source code.
+ */
+declare(strict_types=1);
+
+namespace Ibexa\Tests\CorePersistence\Gateway;
+
+use DateTimeImmutable;
+use Doctrine\Common\Collections\Expr\Comparison;
+use Doctrine\DBAL\Connection;
+use Doctrine\DBAL\Driver\ResultStatement;
+use Doctrine\DBAL\Platforms\PostgreSQL94Platform;
+use Doctrine\DBAL\Query\Expression\ExpressionBuilder;
+use Doctrine\DBAL\Query\QueryBuilder;
+use Doctrine\DBAL\Types\Types;
+use Ibexa\Contracts\CorePersistence\Gateway\AbstractDoctrineDatabase;
+use Ibexa\Contracts\CorePersistence\Gateway\DoctrineSchemaMetadata;
+use Ibexa\Contracts\CorePersistence\Gateway\DoctrineSchemaMetadataInterface;
+use Ibexa\Contracts\CorePersistence\Gateway\DoctrineSchemaMetadataRegistryInterface;
+use PHPUnit\Framework\TestCase;
+
+/**
+ * @covers \Ibexa\Contracts\CorePersistence\Gateway\AbstractDoctrineDatabase
+ */
+final class AbstractDoctrineDatabaseTest extends TestCase
+{
+    /**
+     * @dataProvider provideForDateTimeQueries
+     *
+     * @param array<mixed|\Doctrine\Common\Collections\Expr\Comparison> $query
+     * @param array<array-key, string> $expectedParameters
+     */
+    public function testDateTimeQueries(
+        array $query,
+        string $sql,
+        array $expectedParameters
+    ): void {
+        $connection = $this->createConnectionMock();
+        $database = $this->createDatabaseGateway($connection);
+
+        $result = $this->createMock(ResultStatement::class);
+        $result->expects(self::once())
+            ->method('fetch')
+            ->willReturn(false);
+
+        $connection->expects(self::once())
+            ->method('executeQuery')
+            ->with(
+                self::identicalTo($sql),
+                self::identicalTo($expectedParameters),
+            )
+            ->willReturn($result);
+
+        $database->findBy($query);
+    }
+
+    /**
+     * @return iterable<array{
+     *     array<mixed|\Doctrine\Common\Collections\Expr\Comparison>,
+     *     non-empty-string,
+     *     array<array-key, string>,
+     * }>
+     */
+    public static function provideForDateTimeQueries(): iterable
+    {
+        yield [
+            [
+                new Comparison(
+                    'date_time_immutable_column',
+                    Comparison::LT,
+                    new DateTimeImmutable('2024-01-01 00:00:00'),
+                ),
+            ],
+            str_replace(
+                "\n",
+                ' ',
+                <<<SQL
+                SELECT __foo_table__.id, __foo_table__.date_time_immutable_column
+                FROM __foo_table__ __foo_table__
+                WHERE __foo_table__.date_time_immutable_column < :date_time_immutable_column_0
+                SQL,
+            ),
+            [
+                'date_time_immutable_column_0' => '2024-01-01 00:00:00',
+            ],
+        ];
+
+        yield [
+            [
+                'date_time_immutable_column' => new DateTimeImmutable('2024-01-01 00:00:00'),
+            ],
+            str_replace(
+                "\n",
+                ' ',
+                <<<SQL
+                SELECT __foo_table__.id, __foo_table__.date_time_immutable_column
+                FROM __foo_table__ __foo_table__ WHERE __foo_table__.date_time_immutable_column = ?
+                SQL,
+            ),
+            [
+                1 => '2024-01-01 00:00:00',
+            ],
+        ];
+    }
+
+    /**
+     * @return \Ibexa\Contracts\CorePersistence\Gateway\AbstractDoctrineDatabase<array<mixed>>
+     */
+    private function createDatabaseGateway(Connection $connection): AbstractDoctrineDatabase
+    {
+        $registry = $this->createMock(DoctrineSchemaMetadataRegistryInterface::class);
+
+        $metadata = new DoctrineSchemaMetadata(
+            $connection,
+            null,
+            '__foo_table__',
+            [
+                'id' => Types::INTEGER,
+                'date_time_immutable_column' => Types::DATETIME_IMMUTABLE,
+            ],
+            ['id'],
+        );
+
+        $registry->expects(self::atLeastOnce())
+            ->method('getMetadataForTable')
+            ->with(self::identicalTo('__foo_table__'))
+            ->willReturn($metadata);
+
+        return new class($connection, $registry, $metadata) extends AbstractDoctrineDatabase {
+            private DoctrineSchemaMetadata $metadata;
+
+            public function __construct(
+                Connection $connection,
+                DoctrineSchemaMetadataRegistryInterface $registry,
+                DoctrineSchemaMetadata $metadata
+            ) {
+                parent::__construct($connection, $registry);
+                $this->metadata = $metadata;
+            }
+
+            protected function getTableName(): string
+            {
+                return '__foo_table__';
+            }
+
+            protected function buildMetadata(): DoctrineSchemaMetadataInterface
+            {
+                return $this->metadata;
+            }
+        };
+    }
+
+    /**
+     * @return \Doctrine\DBAL\Connection&\PHPUnit\Framework\MockObject\MockObject
+     */
+    private function createConnectionMock(): Connection
+    {
+        $connection = $this->createMock(Connection::class);
+        $connection->expects(self::once())
+            ->method('createQueryBuilder')
+            ->willReturn(new QueryBuilder($connection));
+
+        $connection->expects(self::atLeastOnce())
+            ->method('getDatabasePlatform')
+            ->willReturn(new PostgreSQL94Platform());
+
+        $connection->expects(self::atLeastOnce())
+            ->method('getExpressionBuilder')
+            ->willReturn(new ExpressionBuilder($connection));
+
+        return $connection;
+    }
+}