Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add processing binary binding type #11734

Open
wants to merge 16 commits into
base: 3.3.x
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 8 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 0 additions & 9 deletions phpstan-dbal3.neon
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,6 @@ parameters:
# Symfony cache supports passing a key prefix to the clear method.
- '/^Method Psr\\Cache\\CacheItemPoolInterface\:\:clear\(\) invoked with 1 parameter, 0 required\.$/'

# We can be certain that those values are not matched.
-
message: '~^Match expression does not handle remaining values:~'
path: src/Persisters/Entity/BasicEntityPersister.php

# DBAL 4 compatibility
-
message: '~^Method Doctrine\\ORM\\Query\\AST\\Functions\\TrimFunction::getTrimMode\(\) never returns .* so it can be removed from the return type\.$~'
Expand Down Expand Up @@ -42,10 +37,6 @@ parameters:
message: '#Expression on left side of \?\? is not nullable.#'
path: src/Mapping/Driver/AttributeDriver.php

-
message: '~^Method Doctrine\\ORM\\Persisters\\Entity\\BasicEntityPersister\:\:getArrayBindingType\(\) never returns .* so it can be removed from the return type\.$~'
path: src/Persisters/Entity/BasicEntityPersister.php

-
message: '~getTypes.*should return~'
path: src/Persisters/Entity/BasicEntityPersister.php
Expand Down
8 changes: 0 additions & 8 deletions phpstan.neon
Original file line number Diff line number Diff line change
Expand Up @@ -7,18 +7,10 @@ parameters:
# Symfony cache supports passing a key prefix to the clear method.
- '/^Method Psr\\Cache\\CacheItemPoolInterface\:\:clear\(\) invoked with 1 parameter, 0 required\.$/'

# We can be certain that those values are not matched.
-
message: '~^Match expression does not handle remaining values:~'
path: src/Persisters/Entity/BasicEntityPersister.php

# DBAL 4 compatibility
-
message: '~^Method Doctrine\\ORM\\Query\\AST\\Functions\\TrimFunction::getTrimMode\(\) never returns .* so it can be removed from the return type\.$~'
path: src/Query/AST/Functions/TrimFunction.php
-
message: '~^Method Doctrine\\ORM\\Persisters\\Entity\\BasicEntityPersister\:\:getArrayBindingType\(\) never returns .* so it can be removed from the return type\.$~'
path: src/Persisters/Entity/BasicEntityPersister.php

# Compatibility with DBAL 3
# See https://github.com/doctrine/dbal/pull/3480
Expand Down
12 changes: 10 additions & 2 deletions src/Persisters/Entity/BasicEntityPersister.php
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
<?php

Check failure on line 1 in src/Persisters/Entity/BasicEntityPersister.php

View workflow job for this annotation

GitHub Actions / Static Analysis with PHPStan (default, phpstan.neon)

Ignored error pattern #^Method Doctrine\\ORM\\Persisters\\Entity\\BasicEntityPersister\:\:getTypes\(\) should return list\<Doctrine\\DBAL\\ArrayParameterType\:\:ASCII\|Doctrine\\DBAL\\ArrayParameterType\:\:BINARY\|Doctrine\\DBAL\\ArrayParameterType\:\:INTEGER\|Doctrine\\DBAL\\ArrayParameterType\:\:STRING\|Doctrine\\DBAL\\ParameterType\:\:ASCII\|Doctrine\\DBAL\\ParameterType\:\:BINARY\|Doctrine\\DBAL\\ParameterType\:\:BOOLEAN\|Doctrine\\DBAL\\ParameterType\:\:INTEGER\|Doctrine\\DBAL\\ParameterType\:\:LARGE_OBJECT\|Doctrine\\DBAL\\ParameterType\:\:NULL\|Doctrine\\DBAL\\ParameterType\:\:STRING\|string\> but returns list\<Doctrine\\DBAL\\ArrayParameterType\:\:ASCII\|Doctrine\\DBAL\\ArrayParameterType\:\:BINARY\|Doctrine\\DBAL\\ArrayParameterType\:\:INTEGER\|Doctrine\\DBAL\\ArrayParameterType\:\:STRING\|int\>\.$# (return.type) in path /home/runner/work/orm/orm/src/Persisters/Entity/BasicEntityPersister.php was not matched in reported errors.

declare(strict_types=1);

Expand All @@ -10,6 +10,7 @@
use Doctrine\Common\Collections\Order;
use Doctrine\DBAL\ArrayParameterType;
use Doctrine\DBAL\Connection;
use Doctrine\DBAL\Exception;
use Doctrine\DBAL\LockMode;
use Doctrine\DBAL\ParameterType;
use Doctrine\DBAL\Platforms\AbstractPlatform;
Expand Down Expand Up @@ -1929,17 +1930,24 @@
return $types;
}

/** @phpstan-return ArrayParameterType::* */
private function getArrayBindingType(ParameterType|int|string $type): ArrayParameterType|int
/**
* @psalm-return ArrayParameterType::*
*
* @throws QueryException
* @throws Exception
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In what situations is this method raising these exception? Is the caller supposed to handle it?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doctrine\DBAL\Types\Type::getType method throws Exception
Screenshot 2024-12-10 at 11 54 45

Copy link
Member

@derrabus derrabus Dec 11, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This answers neither the first nor the second of my questions. 😉

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This method doesn't handle these exceptions directly. I included them to show which ones the method can raise. If you think this is unnecessary, I’ll remove them.

Copy link
Author

@rabikor rabikor Dec 11, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Exception removed. QueryException throws by this method

*/
private function getArrayBindingType(ParameterType|int|string $type): ArrayParameterType

Check failure on line 1939 in src/Persisters/Entity/BasicEntityPersister.php

View workflow job for this annotation

GitHub Actions / Static Analysis with PHPStan (3.8.2, phpstan-dbal3.neon)

PHPDoc tag @return with type int is incompatible with native type Doctrine\DBAL\ArrayParameterType.
{
if (! $type instanceof ParameterType) {
$type = Type::getType((string) $type)->getBindingType();
}

return match ($type) {

Check failure on line 1945 in src/Persisters/Entity/BasicEntityPersister.php

View workflow job for this annotation

GitHub Actions / Static Analysis with PHPStan (3.8.2, phpstan-dbal3.neon)

Method Doctrine\ORM\Persisters\Entity\BasicEntityPersister::getArrayBindingType() should return Doctrine\DBAL\ArrayParameterType but returns int.
ParameterType::STRING => ArrayParameterType::STRING,
ParameterType::INTEGER => ArrayParameterType::INTEGER,
ParameterType::ASCII => ArrayParameterType::ASCII,
ParameterType::BINARY => ArrayParameterType::BINARY,
default => throw new QueryException('Unsupported type for array parameter'),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need the default branch here?

Copy link
Author

@rabikor rabikor Dec 10, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The default is necessary because not all ParameterType cases, such as BOOLEAN, LARGE_OBJECT, or NULL, are supported by ArrayParameterType. The default branch ensures these unsupported types are handled gracefully by throwing a clear exception, preventing unexpected behavior or runtime errors.

};
}

Expand Down
4 changes: 4 additions & 0 deletions tests/Tests/Models/CMS/CmsUser.php
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,10 @@ class CmsUser
#[GeneratedValue]
public $id;

/** @var string */
#[Column(type: 'binary', length: 50, nullable: true)]
public $reference;

/** @var string */
#[Column(type: 'string', length: 50, nullable: true)]
public $status;
Expand Down
51 changes: 27 additions & 24 deletions tests/Tests/ORM/Functional/QueryParameterTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,12 @@
use Doctrine\DBAL\Types\Types;
use Doctrine\Tests\Models\CMS\CmsUser;
use Doctrine\Tests\OrmFunctionalTestCase;
use PHPUnit\Framework\Attributes\DataProvider;
use PHPUnit\Framework\Attributes\Group;

use function hex2bin;
use function sprintf;

#[Group('GH-11278')]
final class QueryParameterTest extends OrmFunctionalTestCase
{
Expand All @@ -22,15 +26,18 @@ protected function setUp(): void

parent::setUp();

$user = new CmsUser();
$user->name = 'John Doe';
$user->username = 'john';
$user2 = new CmsUser();
$user2->name = 'Jane Doe';
$user2->username = 'jane';
$user3 = new CmsUser();
$user3->name = 'Just Bill';
$user3->username = 'bill';
$user = new CmsUser();
$user->reference = hex2bin('1');
$user->name = 'John Doe';
$user->username = 'john';
$user2 = new CmsUser();
$user2->reference = hex2bin('2');
$user2->name = 'Jane Doe';
$user2->username = 'jane';
$user3 = new CmsUser();
$user3->reference = hex2bin('3');
$user3->name = 'Just Bill';
$user3->username = 'bill';

$this->_em->persist($user);
$this->_em->persist($user2);
Expand Down Expand Up @@ -94,31 +101,27 @@ public function testDbalTypeStringInQuery(): void
self::assertSame([['name' => 'John Doe']], $result);
}

public function testArrayParameterTypeInBuilder(): void
#[DataProvider('provideArrayParameters')]
public function testArrayParameterTypeInQuery(string $field, ArrayParameterType $type, array $values): void
{
$result = $this->_em->createQueryBuilder()
->from(CmsUser::class, 'u')
->select('u.name')
->where('u.username IN (:usernames)')
->orderBy('u.username')
->setParameter('usernames', ['john', 'jane'], ArrayParameterType::STRING)
->where(sprintf('u.%s IN (:values)', $field))
->orderBy(sprintf('u.%s', $field))
->setParameter('values', $values, $type)
->getQuery()
->getArrayResult();

self::assertSame([['name' => 'Jane Doe'], ['name' => 'John Doe']], $result);
}

public function testArrayParameterTypeInQuery(): void
public static function provideArrayParameters(): array
{
$result = $this->_em->createQueryBuilder()
->from(CmsUser::class, 'u')
->select('u.name')
->where('u.username IN (:usernames)')
->orderBy('u.username')
->getQuery()
->setParameter('usernames', ['john', 'jane'], ArrayParameterType::STRING)
->getArrayResult();

self::assertSame([['name' => 'Jane Doe'], ['name' => 'John Doe']], $result);
return [
'string' => ['username', ArrayParameterType::STRING, ['john', 'jane']],
'binary' => ['reference', ArrayParameterType::BINARY, [hex2bin('1'), hex2bin('2')]],
'integer' => ['id' => ArrayParameterType::INTEGER, [1, 2]],
];
}
}
Loading