From 5ebe984194403da101b89ddef3a7cd2132056685 Mon Sep 17 00:00:00 2001 From: "Alexander M. Turek" Date: Thu, 24 Feb 2022 11:15:21 +0100 Subject: [PATCH] Add native types to SQLFilter (#9524) --- lib/Doctrine/ORM/Query/Filter/SQLFilter.php | 59 ++++--------- lib/Doctrine/ORM/Query/FilterCollection.php | 83 +++++-------------- psalm-baseline.xml | 6 -- .../Tests/ORM/Functional/SQLFilterTest.php | 75 +++++------------ .../Tests/ORM/Query/FilterCollectionTest.php | 5 +- 5 files changed, 58 insertions(+), 170 deletions(-) diff --git a/lib/Doctrine/ORM/Query/Filter/SQLFilter.php b/lib/Doctrine/ORM/Query/Filter/SQLFilter.php index 583984f6c5c..260b539d6d0 100644 --- a/lib/Doctrine/ORM/Query/Filter/SQLFilter.php +++ b/lib/Doctrine/ORM/Query/Filter/SQLFilter.php @@ -25,39 +25,28 @@ */ abstract class SQLFilter { - /** - * The entity manager. - * - * @var EntityManagerInterface - */ - private $em; - /** * Parameters for the filter. * * @psalm-var array */ - private $parameters = []; + private array $parameters = []; - /** - * @param EntityManagerInterface $em The entity manager. - */ - final public function __construct(EntityManagerInterface $em) - { - $this->em = $em; + final public function __construct( + private EntityManagerInterface $em + ) { } /** * Sets a parameter list that can be used by the filter. * - * @param string $name Name of the parameter. * @param array $values List of parameter values. * @param string $type The parameter type. If specified, the given value will be run through * the type conversion of this type. * * @return $this */ - final public function setParameterList(string $name, array $values, string $type = Types::STRING): self + final public function setParameterList(string $name, array $values, string $type = Types::STRING): static { $this->parameters[$name] = ['value' => $values, 'type' => $type, 'is_list' => true]; @@ -73,15 +62,13 @@ final public function setParameterList(string $name, array $values, string $type /** * Sets a parameter that can be used by the filter. * - * @param string $name Name of the parameter. - * @param mixed $value Value of the parameter. - * @param string|null $type The parameter type. If specified, the given value will be run through - * the type conversion of this type. This is usually not needed for - * strings and numeric types. + * @param string|null $type The parameter type. If specified, the given value will be run through + * the type conversion of this type. This is usually not needed for + * strings and numeric types. * * @return $this */ - final public function setParameter($name, $value, $type = null): self + final public function setParameter(string $name, mixed $value, ?string $type = null): static { if ($type === null) { $type = ParameterTypeInferer::inferType($value); @@ -104,13 +91,11 @@ final public function setParameter($name, $value, $type = null): self * The function is responsible for the right output escaping to use the * value in a query. * - * @param string $name Name of the parameter. - * * @return string The SQL escaped parameter to use in a query. * * @throws InvalidArgumentException */ - final public function getParameter($name) + final public function getParameter(string $name): string { if (! isset($this->parameters[$name])) { throw new InvalidArgumentException("Parameter '" . $name . "' does not exist."); @@ -132,10 +117,6 @@ final public function getParameter($name) * value in a query, separating each entry by comma to inline it into * an IN() query part. * - * @param string $name Name of the parameter. - * - * @return string The SQL escaped parameter to use in a query. - * * @throws InvalidArgumentException */ final public function getParameterList(string $name): string @@ -151,31 +132,26 @@ final public function getParameterList(string $name): string $param = $this->parameters[$name]; $connection = $this->em->getConnection(); - $quoted = array_map(static function ($value) use ($connection, $param) { - return $connection->quote($value, $param['type']); - }, $param['value']); + $quoted = array_map( + static fn (mixed $value): string => (string) $connection->quote($value, $param['type']), + $param['value'] + ); return implode(',', $quoted); } /** * Checks if a parameter was set for the filter. - * - * @param string $name Name of the parameter. - * - * @return bool */ - final public function hasParameter($name) + final public function hasParameter(string $name): bool { return isset($this->parameters[$name]); } /** * Returns as string representation of the SQLFilter parameters (the state). - * - * @return string String representation of the SQLFilter. */ - final public function __toString() + final public function __toString(): string { return serialize($this->parameters); } @@ -191,10 +167,9 @@ final protected function getConnection(): Connection /** * Gets the SQL query part to add to a query. * - * @param string $targetTableAlias * @psalm-param ClassMetadata $targetEntity * * @return string The constraint SQL if there is available, empty string otherwise. */ - abstract public function addFilterConstraint(ClassMetadata $targetEntity, $targetTableAlias); + abstract public function addFilterConstraint(ClassMetadata $targetEntity, string $targetTableAlias): string; } diff --git a/lib/Doctrine/ORM/Query/FilterCollection.php b/lib/Doctrine/ORM/Query/FilterCollection.php index 01c7e2c61c1..c0ec5ecb39d 100644 --- a/lib/Doctrine/ORM/Query/FilterCollection.php +++ b/lib/Doctrine/ORM/Query/FilterCollection.php @@ -29,56 +29,37 @@ class FilterCollection */ public const FILTERS_STATE_DIRTY = 2; - /** - * The used Configuration. - * - * @var Configuration - */ - private $config; - - /** - * The EntityManager that "owns" this FilterCollection instance. - * - * @var EntityManagerInterface - */ - private $em; + private Configuration $config; /** * Instances of enabled filters. * - * @var SQLFilter[] - * @psalm-var array + * @var array */ - private $enabledFilters = []; + private array $enabledFilters = []; - /** - * The filter hash from the last time the query was parsed. - * - * @var string - */ - private $filterHash = ''; + /** The filter hash from the last time the query was parsed. */ + private string $filterHash = ''; /** * The current state of this filter. * - * @var int * @psalm-var self::FILTERS_STATE_* */ - private $filtersState = self::FILTERS_STATE_CLEAN; + private int $filtersState = self::FILTERS_STATE_CLEAN; - public function __construct(EntityManagerInterface $em) - { - $this->em = $em; + public function __construct( + private EntityManagerInterface $em + ) { $this->config = $em->getConfiguration(); } /** * Gets all the enabled filters. * - * @return SQLFilter[] The enabled filters. - * @psalm-return array + * @return array The enabled filters. */ - public function getEnabledFilters() + public function getEnabledFilters(): array { return $this->enabledFilters; } @@ -86,13 +67,9 @@ public function getEnabledFilters() /** * Enables a filter from the collection. * - * @param string $name Name of the filter. - * - * @return SQLFilter The enabled filter. - * * @throws InvalidArgumentException If the filter does not exist. */ - public function enable($name) + public function enable(string $name): SQLFilter { if (! $this->has($name)) { throw new InvalidArgumentException("Filter '" . $name . "' does not exist."); @@ -117,13 +94,9 @@ public function enable($name) /** * Disables a filter. * - * @param string $name Name of the filter. - * - * @return SQLFilter The disabled filter. - * * @throws InvalidArgumentException If the filter does not exist. */ - public function disable($name) + public function disable(string $name): SQLFilter { // Get the filter to return it $filter = $this->getFilter($name); @@ -138,13 +111,9 @@ public function disable($name) /** * Gets an enabled filter from the collection. * - * @param string $name Name of the filter. - * - * @return SQLFilter The filter. - * * @throws InvalidArgumentException If the filter is not enabled. */ - public function getFilter($name) + public function getFilter(string $name): SQLFilter { if (! $this->isEnabled($name)) { throw new InvalidArgumentException("Filter '" . $name . "' is not enabled."); @@ -155,44 +124,32 @@ public function getFilter($name) /** * Checks whether filter with given name is defined. - * - * @param string $name Name of the filter. - * - * @return bool true if the filter exists, false if not. */ - public function has($name) + public function has(string $name): bool { return $this->config->getFilterClassName($name) !== null; } /** * Checks if a filter is enabled. - * - * @param string $name Name of the filter. - * - * @return bool True if the filter is enabled, false otherwise. */ - public function isEnabled($name) + public function isEnabled(string $name): bool { return isset($this->enabledFilters[$name]); } /** * Checks if the filter collection is clean. - * - * @return bool */ - public function isClean() + public function isClean(): bool { return $this->filtersState === self::FILTERS_STATE_CLEAN; } /** * Generates a string of currently enabled filters to use for the cache id. - * - * @return string */ - public function getHash() + public function getHash(): string { // If there are only clean filters, the previous hash can be returned if ($this->filtersState === self::FILTERS_STATE_CLEAN) { @@ -213,10 +170,8 @@ public function getHash() /** * Sets the filter state to dirty. - * - * @return void */ - public function setFiltersStateDirty() + public function setFiltersStateDirty(): void { $this->filtersState = self::FILTERS_STATE_DIRTY; } diff --git a/psalm-baseline.xml b/psalm-baseline.xml index 8b9405e26d7..8b5fa0adc44 100644 --- a/psalm-baseline.xml +++ b/psalm-baseline.xml @@ -1837,12 +1837,6 @@ - - $value - - - static function ($value) use ($connection, $param) { - $this->parameters diff --git a/tests/Doctrine/Tests/ORM/Functional/SQLFilterTest.php b/tests/Doctrine/Tests/ORM/Functional/SQLFilterTest.php index c4ec569d290..4e8511682d3 100644 --- a/tests/Doctrine/Tests/ORM/Functional/SQLFilterTest.php +++ b/tests/Doctrine/Tests/ORM/Functional/SQLFilterTest.php @@ -42,44 +42,19 @@ */ class SQLFilterTest extends OrmFunctionalTestCase { - /** @var int */ - private $userId; - - /** @var int */ - private $userId2; - - /** @var int */ - private $articleId; - - /** @var int */ - private $articleId2; - - /** @var int */ - private $groupId; - - /** @var int */ - private $groupId2; - - /** @var int */ - private $managerId; - - /** @var int */ - private $managerId2; - - /** @var int */ - private $contractId1; - - /** @var int */ - private $contractId2; - - /** @var int */ - private $organizationId; - - /** @var int */ - private $eventId1; - - /** @var int */ - private $eventId2; + private int $userId; + private int $userId2; + private int $articleId; + private int $articleId2; + private int $groupId; + private int $groupId2; + private int $managerId; + private int $managerId2; + private int $contractId1; + private int $contractId2; + private int $organizationId; + private int $eventId1; + private int $eventId2; protected function setUp(): void { @@ -1185,8 +1160,7 @@ public function testRetrieveListAsSingleThrowsException(): void class MySoftDeleteFilter extends SQLFilter { - /** {@inheritDoc} */ - public function addFilterConstraint(ClassMetadata $targetEntity, $targetTableAlias) + public function addFilterConstraint(ClassMetadata $targetEntity, string $targetTableAlias): string { if ($targetEntity->name !== 'MyEntity\SoftDeleteNewsItem') { return ''; @@ -1198,8 +1172,7 @@ public function addFilterConstraint(ClassMetadata $targetEntity, $targetTableAli class MyLocaleFilter extends SQLFilter { - /** {@inheritDoc} */ - public function addFilterConstraint(ClassMetadata $targetEntity, $targetTableAlias) + public function addFilterConstraint(ClassMetadata $targetEntity, string $targetTableAlias): string { if (! in_array('LocaleAware', $targetEntity->reflClass->getInterfaceNames(), true)) { return ''; @@ -1211,8 +1184,7 @@ public function addFilterConstraint(ClassMetadata $targetEntity, $targetTableAli class CMSCountryFilter extends SQLFilter { - /** {@inheritDoc} */ - public function addFilterConstraint(ClassMetadata $targetEntity, $targetTableAlias) + public function addFilterConstraint(ClassMetadata $targetEntity, string $targetTableAlias): string { if ($targetEntity->name !== CmsAddress::class) { return ''; @@ -1224,8 +1196,7 @@ public function addFilterConstraint(ClassMetadata $targetEntity, $targetTableAli class CMSGroupPrefixFilter extends SQLFilter { - /** {@inheritDoc} */ - public function addFilterConstraint(ClassMetadata $targetEntity, $targetTableAlias) + public function addFilterConstraint(ClassMetadata $targetEntity, string $targetTableAlias): string { if ($targetEntity->name !== CmsGroup::class) { return ''; @@ -1237,8 +1208,7 @@ public function addFilterConstraint(ClassMetadata $targetEntity, $targetTableAli class CMSArticleTopicFilter extends SQLFilter { - /** {@inheritDoc} */ - public function addFilterConstraint(ClassMetadata $targetEntity, $targetTableAlias) + public function addFilterConstraint(ClassMetadata $targetEntity, string $targetTableAlias): string { if ($targetEntity->name !== CmsArticle::class) { return ''; @@ -1250,8 +1220,7 @@ public function addFilterConstraint(ClassMetadata $targetEntity, $targetTableAli class CompanyPersonNameFilter extends SQLFilter { - /** {@inheritDoc} */ - public function addFilterConstraint(ClassMetadata $targetEntity, $targetTableAlias, $targetTable = '') + public function addFilterConstraint(ClassMetadata $targetEntity, string $targetTableAlias): string { if ($targetEntity->name !== CompanyPerson::class) { return ''; @@ -1263,8 +1232,7 @@ public function addFilterConstraint(ClassMetadata $targetEntity, $targetTableAli class CompletedContractFilter extends SQLFilter { - /** {@inheritDoc} */ - public function addFilterConstraint(ClassMetadata $targetEntity, $targetTableAlias, $targetTable = '') + public function addFilterConstraint(ClassMetadata $targetEntity, string $targetTableAlias): string { if ($targetEntity->name !== CompanyContract::class) { return ''; @@ -1276,8 +1244,7 @@ public function addFilterConstraint(ClassMetadata $targetEntity, $targetTableAli class CompanyEventFilter extends SQLFilter { - /** {@inheritDoc} */ - public function addFilterConstraint(ClassMetadata $targetEntity, $targetTableAlias, $targetTable = '') + public function addFilterConstraint(ClassMetadata $targetEntity, string $targetTableAlias): string { if ($targetEntity->name !== CompanyEvent::class) { return ''; diff --git a/tests/Doctrine/Tests/ORM/Query/FilterCollectionTest.php b/tests/Doctrine/Tests/ORM/Query/FilterCollectionTest.php index 19b509b4e54..335937972cb 100644 --- a/tests/Doctrine/Tests/ORM/Query/FilterCollectionTest.php +++ b/tests/Doctrine/Tests/ORM/Query/FilterCollectionTest.php @@ -111,10 +111,7 @@ public function testHashing(): void class MyFilter extends SQLFilter { - /** - * {@inheritDoc} - */ - public function addFilterConstraint(ClassMetadata $targetEntity, $targetTableAlias): string + public function addFilterConstraint(ClassMetadata $targetEntity, string $targetTableAlias): string { // getParameter applies quoting automatically return $targetTableAlias . '.id = ' . $this->getParameter('id');