From d30a1513af4fea06fa9f4663a5a005809826535b Mon Sep 17 00:00:00 2001 From: Marc Reichel Date: Thu, 28 Aug 2025 22:33:09 +0200 Subject: [PATCH 1/2] feat: Add strict types, new conditions, and extensive test coverage - Enforced strict types across source code. - Introduced new condition classes including `EqualsNotCondition`, `NotEmptyCondition`, `NotInCondition`, `NotLikeCondition`, and `IsNotNullCondition`. - Added PHPUnit tests for various condition classes. - Enhanced code quality and coverage through updates to Rector, Pint, and Composer scripts. --- .github/workflows/rector.yml | 27 +++++++++ composer.json | 10 ++-- pint.json | 1 + rector.php | 28 +++------ src/Attribute/OneToMany.php | 2 + src/Attribute/TableIndex.php | 2 + src/Attribute/TablePrimary.php | 2 + src/Collection.php | 4 +- src/Comparator.php | 2 + src/Condition.php | 30 +++++++--- src/Condition/CompositeCondition.php | 19 +++--- src/Condition/EmptyCondition.php | 30 ++++++++-- src/Condition/EqualsCondition.php | 22 +++++-- src/Condition/EqualsNotCondition.php | 13 ++++ src/Condition/GreaterCondition.php | 19 +++--- src/Condition/InCondition.php | 59 +++++++------------ src/Condition/IsNotNullCondition.php | 13 ++++ src/Condition/IsNullCondition.php | 18 +++--- src/Condition/LikeCondition.php | 19 +++--- src/Condition/LowerCondition.php | 19 +++--- src/Condition/NotEmptyCondition.php | 13 ++++ src/Condition/NotInCondition.php | 13 ++++ src/Condition/NotLikeCondition.php | 13 ++++ src/ConditionInterface.php | 2 + src/Conjunction.php | 2 + src/Converter.php | 2 + src/EntityInterface.php | 2 + src/EntityManager.php | 9 ++- src/EntityMeta.php | 6 ++ src/Exception/OrmException.php | 2 + src/FieldMapper.php | 19 +++--- src/OrderBy.php | 14 +++-- src/OrderByInterface.php | 2 + src/QueryBuilder.php | 18 +++--- src/SchemaManager.php | 14 +++-- src/TypeConverterInterface.php | 2 + tests/Condition/CompositeConditionTest.php | 51 ++++++++++++++++ tests/Condition/EmptyConditionTest.php | 31 ++++++++++ tests/Condition/EqualsConditionTest.php | 31 ++++++++++ tests/Condition/GreaterConditionTest.php | 30 ++++++++++ tests/Condition/InConditionTest.php | 51 ++++++++++++++++ tests/Condition/IsNullConditionTest.php | 31 ++++++++++ tests/Condition/LikeConditionTest.php | 31 ++++++++++ tests/Condition/LowerConditionTest.php | 30 ++++++++++ tests/EntityManagerTest.php | 3 + tests/EntityManagerTestCase.php | 8 +-- tests/FieldMapper/TestInvalidModel.php | 11 ++++ tests/FieldMapper/TestModel.php | 8 ++- .../TestModelWithTooLongRelation.php | 41 +++++++++++++ tests/FieldMapper/TestParent.php | 2 + tests/FieldMapper/TestSubTestUser.php | 25 ++++++++ tests/FieldMapper/TestUserModel.php | 26 ++++++++ tests/FieldMapperTest.php | 2 + tests/QueryBuilderTest.php | 48 +++++++++++++-- 54 files changed, 771 insertions(+), 161 deletions(-) create mode 100644 .github/workflows/rector.yml create mode 100644 src/Condition/EqualsNotCondition.php create mode 100644 src/Condition/IsNotNullCondition.php create mode 100644 src/Condition/NotEmptyCondition.php create mode 100644 src/Condition/NotInCondition.php create mode 100644 src/Condition/NotLikeCondition.php create mode 100644 tests/Condition/CompositeConditionTest.php create mode 100644 tests/Condition/EmptyConditionTest.php create mode 100644 tests/Condition/EqualsConditionTest.php create mode 100644 tests/Condition/GreaterConditionTest.php create mode 100644 tests/Condition/InConditionTest.php create mode 100644 tests/Condition/IsNullConditionTest.php create mode 100644 tests/Condition/LikeConditionTest.php create mode 100644 tests/Condition/LowerConditionTest.php create mode 100644 tests/FieldMapper/TestInvalidModel.php create mode 100644 tests/FieldMapper/TestModelWithTooLongRelation.php create mode 100644 tests/FieldMapper/TestSubTestUser.php create mode 100644 tests/FieldMapper/TestUserModel.php diff --git a/.github/workflows/rector.yml b/.github/workflows/rector.yml new file mode 100644 index 0000000..bfba6bf --- /dev/null +++ b/.github/workflows/rector.yml @@ -0,0 +1,27 @@ +name: Rector + +on: + - pull_request + +concurrency: + group: ${{ github.workflow }}-${{ github.ref }} + cancel-in-progress: true + +jobs: + pint: + name: Rector + runs-on: ubuntu-latest + steps: + - name: Checkout + uses: actions/checkout@v4 + - name: Install PHP + uses: shivammathur/setup-php@v2 + with: + php-version: "8.4" + coverage: none + env: + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + - name: Composer install + run: composer install --no-interaction --no-ansi --no-progress + - name: Run Rector + run: ./vendor/bin/rector process --dry-run diff --git a/composer.json b/composer.json index f528f56..e2c3d07 100644 --- a/composer.json +++ b/composer.json @@ -5,11 +5,11 @@ "description": "", "scripts": { "phpstan": "php ./vendor/bin/phpstan analyse --memory-limit=4G", - "pint": "./vendor/bin/pint --test -v", - "pint:fix": "./vendor/bin/pint", - "test": "./vendor/bin/pest", - "test:coverage": "XDEBUG_MODE=coverage ./vendor/bin/pest --coverage --min=63.2", - "test:type-coverage": "XDEBUG_MODE=coverage ./vendor/bin/pest --type-coverage --min=99.8" + "pint": "./vendor/bin/pint --test -v --parallel", + "pint:fix": "./vendor/bin/pint -v --parallel", + "test": "./vendor/bin/pest --parallel", + "test:coverage": "XDEBUG_MODE=coverage ./vendor/bin/pest --coverage --parallel --min=84.6", + "test:type-coverage": "XDEBUG_MODE=coverage ./vendor/bin/pest --type-coverage --min=100" }, "authors": [ { diff --git a/pint.json b/pint.json index c3d7675..3a12c4e 100644 --- a/pint.json +++ b/pint.json @@ -18,6 +18,7 @@ "concat_space": { "spacing": "one" }, + "declare_strict_types": true, "fully_qualified_strict_types": true, "function_to_constant": true, "general_phpdoc_annotation_remove": { diff --git a/rector.php b/rector.php index 0d862f3..2abb8d5 100644 --- a/rector.php +++ b/rector.php @@ -5,32 +5,22 @@ use Rector\Config\RectorConfig; return RectorConfig::configure() + ->withPreparedSets( + deadCode: true, + codeQuality: true, + codingStyle: true, + typeDeclarations: true, + earlyReturn: true, + strictBooleans: true, + ) ->withPhpSets() ->withAttributesSets(phpunit: true) ->withRules([ - Rector\CodeQuality\Rector\Ternary\ArrayKeyExistsTernaryThenValueToCoalescingRector::class, - Rector\CodeQuality\Rector\NullsafeMethodCall\CleanupUnneededNullsafeOperatorRector::class, - Rector\CodeQuality\Rector\ClassMethod\InlineArrayReturnAssignRector::class, - Rector\CodeQuality\Rector\Ternary\UnnecessaryTernaryExpressionRector::class, - Rector\DeadCode\Rector\Foreach_\RemoveUnusedForeachKeyRector::class, - Rector\TypeDeclaration\Rector\ClassMethod\ReturnTypeFromStrictFluentReturnRector::class, - Rector\Php80\Rector\Class_\StringableForToStringRector::class, Rector\CodingStyle\Rector\ArrowFunction\StaticArrowFunctionRector::class, Rector\CodingStyle\Rector\Closure\StaticClosureRector::class, - Rector\DeadCode\Rector\Node\RemoveNonExistingVarAnnotationRector::class, - Rector\DeadCode\Rector\ClassMethod\RemoveUnusedPrivateMethodParameterRector::class, - Rector\TypeDeclaration\Rector\ClassMethod\BoolReturnTypeFromBooleanStrictReturnsRector::class, - Rector\TypeDeclaration\Rector\ClassMethod\ReturnTypeFromReturnNewRector::class, - Rector\TypeDeclaration\Rector\ClassMethod\ParamTypeByMethodCallTypeRector::class, - Rector\TypeDeclaration\Rector\ClassMethod\NumericReturnTypeFromStrictScalarReturnsRector::class, - Rector\TypeDeclaration\Rector\ClassMethod\AddMethodCallBasedStrictParamTypeRector::class, - Rector\CodeQuality\Rector\If_\ExplicitBoolCompareRector::class, - Rector\CodeQuality\Rector\Foreach_\ForeachItemsAssignToEmptyArrayToAssignRector::class, - Rector\CodeQuality\Rector\Foreach_\ForeachToInArrayRector::class, - Rector\CodeQuality\Rector\BooleanAnd\RemoveUselessIsObjectCheckRector::class, ]) ->withPaths([ __DIR__ . '/src', __DIR__ . '/tests', ]) - ->withTypeCoverageLevel(0); + ->withImportNames(removeUnusedImports: true); diff --git a/src/Attribute/OneToMany.php b/src/Attribute/OneToMany.php index 02a7a1f..de7a00d 100644 --- a/src/Attribute/OneToMany.php +++ b/src/Attribute/OneToMany.php @@ -1,5 +1,7 @@ where); + return $this->where === '' || $this->where === '0'; } /** @@ -56,29 +58,39 @@ public function isEmpty(): bool final public static function forValue(mixed $value, string $tableColumn, ?Comparator $comparator = null): ?ConditionInterface { if (is_string($value)) { - if ($comparator === null || $comparator === Comparator::LIKE) { + if (!$comparator instanceof Comparator || $comparator === Comparator::LIKE) { return new LikeCondition($tableColumn, '%' . $value . '%'); } return new Condition($tableColumn . ' ' . $comparator->toSql() . ' ?', [$value]); - } elseif (is_int($value) || is_float($value)) { - if ($comparator === null || $comparator === Comparator::EQUAL) { + } + + if (is_int($value) || is_float($value)) { + if (!$comparator instanceof Comparator || $comparator === Comparator::EQUAL) { return new EqualsCondition($tableColumn, $value); } return new Condition($tableColumn . ' ' . $comparator->toSql() . ' ?', [$value]); - } elseif (is_bool($value)) { - if ($comparator === null || $comparator === Comparator::EQUAL) { + } + + if (is_bool($value)) { + if (!$comparator instanceof Comparator || $comparator === Comparator::EQUAL) { return new EqualsCondition($tableColumn, $value ? 1 : 0); } return new Condition($tableColumn . ' ' . $comparator->toSql() . ' ?', [$value]); - } elseif (null === $value) { + } + + if (null === $value) { return new IsNullCondition($tableColumn, $comparator === Comparator::IS_NOT_NULL); - } elseif (is_array($value)) { + } + + if (is_array($value)) { if ($comparator === Comparator::IN_OR_EMPTY) { return new CompositeCondition([new InCondition($tableColumn, $value), new EmptyCondition($tableColumn)], Conjunction::OR); - } elseif ($comparator === Comparator::NOT_IN_OR_EMPTY) { + } + + if ($comparator === Comparator::NOT_IN_OR_EMPTY) { return new CompositeCondition([new InCondition($tableColumn, $value, true), new EmptyCondition($tableColumn)], Conjunction::OR); } diff --git a/src/Condition/CompositeCondition.php b/src/Condition/CompositeCondition.php index bb26e7e..e6cd997 100644 --- a/src/Condition/CompositeCondition.php +++ b/src/Condition/CompositeCondition.php @@ -1,16 +1,15 @@ ) AND () AND () ) - * ( () OR () OR () ). + * An orm condition to store several orm conditions. + * They will connect via the given conjunction. */ class CompositeCondition implements ConditionInterface { @@ -44,7 +43,7 @@ public function addCondition(ConditionInterface $condition): self public function hasConditions(): bool { - return count($this->conditions) > 0; + return $this->conditions !== []; } public function getWhere(): string @@ -58,13 +57,9 @@ public function getWhere(): string } $result = ''; - if (count($where) > 0) { + if ($where !== []) { $result = implode(') ' . $this->conjunction->toSql() . ' (', $where); - if (count($where) == 1) { - $result = '(' . $result . ')'; - } else { - $result = '( (' . $result . ') )'; - } + $result = count($where) === 1 ? '(' . $result . ')' : '( (' . $result . ') )'; } return $result; diff --git a/src/Condition/EmptyCondition.php b/src/Condition/EmptyCondition.php index d545590..69d6fac 100644 --- a/src/Condition/EmptyCondition.php +++ b/src/Condition/EmptyCondition.php @@ -4,27 +4,45 @@ namespace Artemeon\Orm\Condition; +use Artemeon\Orm\Comparator; use Artemeon\Orm\ConditionInterface; +use Artemeon\Orm\Conjunction; use function sprintf; -class EmptyCondition implements ConditionInterface +readonly class EmptyCondition implements ConditionInterface { - public function __construct(private string $columnName, private bool $negated = false) - { + public function __construct( + private string $columnName, + private bool $negated = false, + ) { } public function getParams(): array { - return []; + return ['']; } public function getWhere(): string { if ($this->negated) { - return sprintf('%s IS NOT NULL AND %s != ?', $this->columnName, ''); + return sprintf( + '%s %s %s %s %s ?', + $this->columnName, + Comparator::IS_NOT_NULL->toSql(), + Conjunction::AND->toSql(), + $this->columnName, + Comparator::NOT_EQUAL->toSql(), + ); } - return sprintf('%s IS NULL OR %s = ?', $this->columnName, ''); + return sprintf( + '%s %s %s %s %s ?', + $this->columnName, + Comparator::IS_NULL->toSql(), + Conjunction::OR->toSql(), + $this->columnName, + Comparator::EQUAL->toSql(), + ); } } diff --git a/src/Condition/EqualsCondition.php b/src/Condition/EqualsCondition.php index 0dbc2c9..01ee800 100644 --- a/src/Condition/EqualsCondition.php +++ b/src/Condition/EqualsCondition.php @@ -4,14 +4,18 @@ namespace Artemeon\Orm\Condition; +use Artemeon\Orm\Comparator; use Artemeon\Orm\ConditionInterface; use function sprintf; -class EqualsCondition implements ConditionInterface +readonly class EqualsCondition implements ConditionInterface { - public function __construct(private readonly string $columnName, private readonly mixed $value, private readonly bool $negated = false) - { + public function __construct( + private string $columnName, + private mixed $value, + private bool $negated = false, + ) { } public function getParams(): array @@ -22,9 +26,17 @@ public function getParams(): array public function getWhere(): string { if ($this->negated) { - return sprintf('%s != ?', $this->columnName); + return sprintf( + '%s %s ?', + $this->columnName, + Comparator::NOT_EQUAL->toSql(), + ); } - return sprintf('%s = ?', $this->columnName); + return sprintf( + '%s %s ?', + $this->columnName, + Comparator::EQUAL->toSql(), + ); } } diff --git a/src/Condition/EqualsNotCondition.php b/src/Condition/EqualsNotCondition.php new file mode 100644 index 0000000..42a79d0 --- /dev/null +++ b/src/Condition/EqualsNotCondition.php @@ -0,0 +1,13 @@ +inclusive) { - return sprintf('%s >= ?', $this->columnName); - } + return sprintf('%s %s ?', $this->columnName, $this->getComparator()->toSql()); + } - return sprintf('%s > ?', $this->columnName); + private function getComparator(): Comparator + { + return $this->inclusive ? Comparator::GREATER_THEN_EQUALS : Comparator::GREATER_THEN; } } diff --git a/src/Condition/InCondition.php b/src/Condition/InCondition.php index a1c2e85..56213d0 100644 --- a/src/Condition/InCondition.php +++ b/src/Condition/InCondition.php @@ -1,49 +1,36 @@ IN ()". */ -class InCondition extends Condition +readonly class InCondition implements ConditionInterface { /** * @internal */ public const int MAX_IN_VALUES = 950; - public function __construct(protected string $columnName, array $params, protected bool $negated = false) - { - parent::__construct('', $params); - } - - /** - * @throws OrmException - */ - #[Override] - public function setParams(array $params): void - { - throw new OrmException('Setting params for property IN restrictions is not supported'); + public function __construct( + private string $columnName, + private array $params, + private bool $negated = false, + ) { } - /** - * @throws OrmException - */ - #[Override] - public function setWhere(string $where): void + public function getParams(): array { - throw new OrmException('Setting a where restriction for property IN restrictions is not supported'); + return $this->params; } - /** - * Here comes the magic, generation a where restriction out of the passed property name and the comparator. - */ - #[Override] public function getWhere(): string { return $this->getInStatement($this->columnName); @@ -51,11 +38,11 @@ public function getWhere(): string protected function getInStatement(string $columnName): string { - if (count($this->params) === 0) { + if ($this->params === []) { return ''; } - $operator = $this->negated ? 'NOT IN' : 'IN'; + $operator = $this->negated ? Comparator::NOT_IN : Comparator::IN; if (count($this->params) > self::MAX_IN_VALUES) { $count = ceil(count($this->params) / self::MAX_IN_VALUES); @@ -63,22 +50,20 @@ protected function getInStatement(string $columnName): string for ($i = 0; $i < $count; $i++) { $params = array_slice($this->params, $i * self::MAX_IN_VALUES, self::MAX_IN_VALUES); - $paramsPlaceholder = array_map(static fn (mixed $value) => '?', $params); + $paramsPlaceholder = array_map(static fn (mixed $value): string => '?', $params); $placeholder = implode(',', $paramsPlaceholder); - if (! empty($placeholder)) { - $parts[] = "{$columnName} {$operator} ({$placeholder})"; + if ($placeholder !== '') { + $parts[] = sprintf('%s %s (%s)', $columnName, $operator->toSql(), $placeholder); } } - if (count($parts) > 0) { - return '(' . implode(' OR ', $parts) . ')'; + if ($parts !== []) { + return '(' . implode(' ' . Conjunction::OR->toSql() . ' ', $parts) . ')'; } } else { - $placeholder = trim(str_repeat('?,', count($this->params)), ','); + $placeholder = implode(',', array_map(static fn (mixed $value): string => '?', $this->params)); - if ($placeholder !== '' && $placeholder !== '0') { - return "{$columnName} {$operator} ({$placeholder})"; - } + return sprintf('%s %s (%s)', $columnName, $operator->toSql(), $placeholder); } return ''; diff --git a/src/Condition/IsNotNullCondition.php b/src/Condition/IsNotNullCondition.php new file mode 100644 index 0000000..0c2497b --- /dev/null +++ b/src/Condition/IsNotNullCondition.php @@ -0,0 +1,13 @@ +negated) { - return sprintf('%s IS NOT NULL', $this->columnName); - } + return sprintf('%s %s', $this->columnName, $this->getComparator()->toSql()); + } - return sprintf('%s IS NULL', $this->columnName); + private function getComparator(): Comparator + { + return $this->negated ? Comparator::IS_NOT_NULL : Comparator::IS_NULL; } } diff --git a/src/Condition/LikeCondition.php b/src/Condition/LikeCondition.php index 8435698..9cd67dc 100644 --- a/src/Condition/LikeCondition.php +++ b/src/Condition/LikeCondition.php @@ -4,14 +4,18 @@ namespace Artemeon\Orm\Condition; +use Artemeon\Orm\Comparator; use Artemeon\Orm\ConditionInterface; use function sprintf; -class LikeCondition implements ConditionInterface +readonly class LikeCondition implements ConditionInterface { - public function __construct(private readonly string $columnName, private readonly mixed $value, private readonly bool $negated = false) - { + public function __construct( + private string $columnName, + private mixed $value, + private bool $negated = false, + ) { } public function getParams(): array @@ -21,10 +25,11 @@ public function getParams(): array public function getWhere(): string { - if ($this->negated) { - return sprintf('%s NOT LIKE ?', $this->columnName); - } + return sprintf('%s %s ?', $this->columnName, $this->getComparator()->toSql()); + } - return sprintf('%s LIKE ?', $this->columnName); + private function getComparator(): Comparator + { + return $this->negated ? Comparator::NOT_LIKE : Comparator::LIKE; } } diff --git a/src/Condition/LowerCondition.php b/src/Condition/LowerCondition.php index d0d0fa6..b170943 100644 --- a/src/Condition/LowerCondition.php +++ b/src/Condition/LowerCondition.php @@ -4,14 +4,18 @@ namespace Artemeon\Orm\Condition; +use Artemeon\Orm\Comparator; use Artemeon\Orm\ConditionInterface; use function sprintf; -class LowerCondition implements ConditionInterface +readonly class LowerCondition implements ConditionInterface { - public function __construct(private readonly string $columnName, private readonly mixed $value, private readonly bool $inclusive = false) - { + public function __construct( + private string $columnName, + private mixed $value, + private bool $inclusive = false, + ) { } public function getParams(): array @@ -21,10 +25,11 @@ public function getParams(): array public function getWhere(): string { - if ($this->inclusive) { - return sprintf('%s <= ?', $this->columnName); - } + return sprintf('%s %s ?', $this->columnName, $this->getComparator()->toSql()); + } - return sprintf('%s < ?', $this->columnName); + private function getComparator(): Comparator + { + return $this->inclusive ? Comparator::LESS_THEN_EQUALS : Comparator::LESS_THEN; } } diff --git a/src/Condition/NotEmptyCondition.php b/src/Condition/NotEmptyCondition.php new file mode 100644 index 0000000..4329935 --- /dev/null +++ b/src/Condition/NotEmptyCondition.php @@ -0,0 +1,13 @@ +getQuery($targetClass, $conditions, $sorting); $row = $this->connection->fetchAssociative($query, $params); - if (empty($row)) { + if ($row === [] || $row === false) { return null; } @@ -96,7 +98,7 @@ private function getQuery(string $targetClass, array $conditions = [], array $so $params = array_merge($params, $condition->getParams()); } - if (count($sorting) > 0) { + if ($sorting !== []) { $query .= ' ORDER BY '; foreach ($sorting as $sort) { if (! $sort instanceof OrderBy) { @@ -172,6 +174,7 @@ public function update(EntityInterface $entity): void if (! isset($data[$tableName])) { $data[$tableName] = []; } + if (! isset($identifiers[$tableName])) { $identifiers[$tableName] = []; } @@ -267,7 +270,7 @@ private function handleRelations(EntityInterface $entity, array $relations): voi $this->connection->delete($relationTable, [$sourceColumn => $sourcePrimaryId]); foreach ($collection as $relationEntity) { $relationEntityId = $this->entityMeta->getPrimaryId($relationEntity); - if (empty($relationEntityId)) { + if ($relationEntityId === null || $relationEntityId === '' || $relationEntityId === '0') { $relationEntityId = $this->insert($relationEntity); } diff --git a/src/EntityMeta.php b/src/EntityMeta.php index 339a44e..11ac3ba 100644 --- a/src/EntityMeta.php +++ b/src/EntityMeta.php @@ -1,5 +1,7 @@ cache->has($cacheKey)) { return $this->cache->get($cacheKey); } + $types = $this->getTypesFromEntity($entityClass); $this->cache->set($cacheKey, $types); @@ -75,6 +78,7 @@ public function getTableNames(string $entityClass): array if ($this->cache->has($cacheKey)) { return $this->cache->get($cacheKey); } + $types = $this->getTableNamesFromEntity($entityClass); $this->cache->set($cacheKey, $types); @@ -90,11 +94,13 @@ private function getTableNamesFromEntity(string $entityClass): array if ($tableName instanceof TableName) { $result[$class->getName()] = $tableName->tableName; } + while ($parentClass = $class->getParentClass()) { $tableName = $this->findTableNameAttribute($parentClass); if ($tableName instanceof TableName) { $result[$parentClass->getName()] = $tableName->tableName; } + $class = $parentClass; } diff --git a/src/Exception/OrmException.php b/src/Exception/OrmException.php index d526c9c..908bb46 100644 --- a/src/Exception/OrmException.php +++ b/src/Exception/OrmException.php @@ -1,5 +1,7 @@ queryBuilder = new QueryBuilder($this->connection, $this->entityMeta); } @@ -27,7 +32,7 @@ public function map(EntityInterface $entity, array $row): void $properties = $this->entityMeta->getProperties($entity::class); foreach ($properties as $config) { if ($config[0] === EntityMeta::TYPE_FIELD) { - [$fieldType, $class, $setter, $getter, $columnName, $dataType, $type, $length, $nullable, $default, $isPrimary] = $config; + [,, $setter,, $columnName, $dataType] = $config; if (! isset($row[$columnName])) { continue; @@ -35,7 +40,7 @@ public function map(EntityInterface $entity, array $row): void $value = $this->converter->toPHPType($row[$columnName], $dataType); } elseif ($config[0] === EntityMeta::TYPE_ONE_TO_MANY) { - [$type, $class, $setter, $getter, $relationTable, $sourceColumn, $targetColumn, $types] = $config; + [,, $setter,, $relationTable, $sourceColumn,, $types] = $config; $value = new Collection($relationTable, $sourceColumn, $types, $row[$sourcePrimaryColumn], $this->connection, $this, $this->queryBuilder); } else { diff --git a/src/OrderBy.php b/src/OrderBy.php index 99246e6..8ec9970 100644 --- a/src/OrderBy.php +++ b/src/OrderBy.php @@ -1,20 +1,24 @@ $this->orderBy = ' ' . $value . ' '; + } public function __construct(string $orderBy) { - $this->orderBy = ' ' . $orderBy . ' '; + $this->orderBy = $orderBy; } public function setOrderBy(string $orderBy): void diff --git a/src/OrderByInterface.php b/src/OrderByInterface.php index 9ca0ed5..2c10f37 100644 --- a/src/OrderByInterface.php +++ b/src/OrderByInterface.php @@ -1,5 +1,7 @@ entityMeta->getTableNames($entityClass); - if (count($targetTables) == 0) { + if ($targetTables === []) { throw new OrmException('Entity ' . $entityClass . ' has no target table'); } @@ -35,12 +39,10 @@ public function buildFrom(string $entityClass, ?string $joinColumn = null): stri } else { $parts[] = 'FROM ' . $enclosedTable . ' AS ' . $enclosedTable; } + } elseif (in_array($tableName, $this->blockedTableAlias, true)) { + $parts[] = 'INNER JOIN ' . $enclosedTable . ' ON ' . $primaryColumn . ' = ' . $firstPrimaryKey; } else { - if (in_array($tableName, $this->blockedTableAlias)) { - $parts[] = 'INNER JOIN ' . $enclosedTable . ' ON ' . $primaryColumn . ' = ' . $firstPrimaryKey; - } else { - $parts[] = 'INNER JOIN ' . $enclosedTable . ' AS ' . $enclosedTable . ' ON ' . $enclosedTable . '.' . $primaryColumn . ' = ' . $firstPrimaryKey; - } + $parts[] = 'INNER JOIN ' . $enclosedTable . ' AS ' . $enclosedTable . ' ON ' . $enclosedTable . '.' . $primaryColumn . ' = ' . $firstPrimaryKey; } } diff --git a/src/SchemaManager.php b/src/SchemaManager.php index 2f4adc5..2a69086 100644 --- a/src/SchemaManager.php +++ b/src/SchemaManager.php @@ -1,15 +1,19 @@ [DataType::CHAR20, false], diff --git a/src/TypeConverterInterface.php b/src/TypeConverterInterface.php index 7d649ae..a1c49a9 100644 --- a/src/TypeConverterInterface.php +++ b/src/TypeConverterInterface.php @@ -1,5 +1,7 @@ getWhere()); + self::assertSame(['bar'], $condition->getParams()); + } + + public static function multipleConditionsProvider(): iterable + { + foreach (Conjunction::cases() as $conjunction) { + yield [$conjunction]; + } + } + + #[DataProvider('multipleConditionsProvider')] + public function testMultipleConditions(Conjunction $conjunction): void + { + $condition = new CompositeCondition([ + new EqualsCondition('foo', 'bar'), + ]); + + $condition->setConjunction($conjunction); + + $condition->addCondition(new EqualsCondition('baz', 'qux')); + + self::assertTrue($condition->hasConditions()); + self::assertSame('( (foo = ?) ' . $conjunction->toSql() . ' (baz = ?) )', $condition->getWhere()); + self::assertSame(['bar', 'qux'], $condition->getParams()); + self::assertSame($conjunction, $condition->getConjunction()); + } +} diff --git a/tests/Condition/EmptyConditionTest.php b/tests/Condition/EmptyConditionTest.php new file mode 100644 index 0000000..60e224d --- /dev/null +++ b/tests/Condition/EmptyConditionTest.php @@ -0,0 +1,31 @@ +getWhere()); + self::assertSame([''], $condition->getParams()); + } + + public function testNegatedCondition(): void + { + $condition = new NotEmptyCondition('foo'); + + self::assertSame('foo IS NOT NULL AND foo != ?', $condition->getWhere()); + self::assertSame([''], $condition->getParams()); + } +} diff --git a/tests/Condition/EqualsConditionTest.php b/tests/Condition/EqualsConditionTest.php new file mode 100644 index 0000000..f3571c2 --- /dev/null +++ b/tests/Condition/EqualsConditionTest.php @@ -0,0 +1,31 @@ +getWhere()); + self::assertSame(['bar'], $condition->getParams()); + } + + public function testNegatedCondition(): void + { + $condition = new EqualsNotCondition('foo', 'bar'); + + self::assertSame('foo != ?', $condition->getWhere()); + self::assertSame(['bar'], $condition->getParams()); + } +} diff --git a/tests/Condition/GreaterConditionTest.php b/tests/Condition/GreaterConditionTest.php new file mode 100644 index 0000000..0eb947c --- /dev/null +++ b/tests/Condition/GreaterConditionTest.php @@ -0,0 +1,30 @@ + ?', $condition->getWhere()); + self::assertSame(['bar'], $condition->getParams()); + } + + public function testInclusiveCondition(): void + { + $condition = new GreaterCondition('foo', 'bar', true); + + self::assertSame('foo >= ?', $condition->getWhere()); + self::assertSame(['bar'], $condition->getParams()); + } +} diff --git a/tests/Condition/InConditionTest.php b/tests/Condition/InConditionTest.php new file mode 100644 index 0000000..d174c40 --- /dev/null +++ b/tests/Condition/InConditionTest.php @@ -0,0 +1,51 @@ +getWhere()); + self::assertSame(['bar', 'baz'], $condition->getParams()); + } + + public function testNegatedCondition(): void + { + $condition = new NotInCondition('foo', ['bar', 'baz']); + + self::assertSame('foo NOT IN (?,?)', $condition->getWhere()); + self::assertSame(['bar', 'baz'], $condition->getParams()); + } + + public function testManyValues(): void + { + $range = range(1, InCondition::MAX_IN_VALUES + 1); + + $condition = new InCondition('foo', $range); + + $first = implode(',', array_map(static fn (int $step): string => '?', range(1, InCondition::MAX_IN_VALUES))); + $second = implode(',', array_map(static fn (int $step): string => '?', range(InCondition::MAX_IN_VALUES + 1, InCondition::MAX_IN_VALUES + 1))); + + self::assertSame('(foo IN (' . $first . ') OR foo IN (' . $second . '))', $condition->getWhere()); + self::assertSame($range, $condition->getParams()); + } + + public function testEmptyValues(): void + { + $condition = new InCondition('foo', []); + + self::assertSame('', $condition->getWhere()); + } +} diff --git a/tests/Condition/IsNullConditionTest.php b/tests/Condition/IsNullConditionTest.php new file mode 100644 index 0000000..72a3dc5 --- /dev/null +++ b/tests/Condition/IsNullConditionTest.php @@ -0,0 +1,31 @@ +getWhere()); + self::assertSame([], $condition->getParams()); + } + + public function testNegatedCondition(): void + { + $condition = new IsNotNullCondition('foo'); + + self::assertSame('foo IS NOT NULL', $condition->getWhere()); + self::assertSame([], $condition->getParams()); + } +} diff --git a/tests/Condition/LikeConditionTest.php b/tests/Condition/LikeConditionTest.php new file mode 100644 index 0000000..ecb7b38 --- /dev/null +++ b/tests/Condition/LikeConditionTest.php @@ -0,0 +1,31 @@ +getWhere()); + self::assertSame(['%bar%'], $condition->getParams()); + } + + public function testNegatedCondition(): void + { + $condition = new NotLikeCondition('foo', '%bar%'); + + self::assertSame('foo NOT LIKE ?', $condition->getWhere()); + self::assertSame(['%bar%'], $condition->getParams()); + } +} diff --git a/tests/Condition/LowerConditionTest.php b/tests/Condition/LowerConditionTest.php new file mode 100644 index 0000000..957ef5e --- /dev/null +++ b/tests/Condition/LowerConditionTest.php @@ -0,0 +1,30 @@ +getWhere()); + self::assertSame(['bar'], $condition->getParams()); + } + + public function testInclusiveCondition(): void + { + $condition = new LowerCondition('foo', 'bar', true); + + self::assertSame('foo <= ?', $condition->getWhere()); + self::assertSame(['bar'], $condition->getParams()); + } +} diff --git a/tests/EntityManagerTest.php b/tests/EntityManagerTest.php index e7d68d5..9c1821b 100644 --- a/tests/EntityManagerTest.php +++ b/tests/EntityManagerTest.php @@ -1,5 +1,7 @@ setOwner('foobar'); + $collection = new ArrayCollection(); $collection->add($relation); diff --git a/tests/EntityManagerTestCase.php b/tests/EntityManagerTestCase.php index 1a3618d..cfb391e 100644 --- a/tests/EntityManagerTestCase.php +++ b/tests/EntityManagerTestCase.php @@ -39,7 +39,7 @@ protected function setUp(): void protected function getConnection(): ConnectionInterface { - if (self::$connection !== null) { + if (self::$connection instanceof ConnectionInterface) { return self::$connection; } @@ -58,7 +58,7 @@ protected function getConnection(): ConnectionInterface protected function getEntityManager(): EntityManager { - if (self::$entityManager !== null) { + if (self::$entityManager instanceof EntityManager) { return self::$entityManager; } @@ -72,7 +72,7 @@ protected function getEntityManager(): EntityManager protected function getSchemaManager(): SchemaManager { - if (self::$schemaManager !== null) { + if (self::$schemaManager instanceof SchemaManager) { return self::$schemaManager; } @@ -83,7 +83,7 @@ protected function getSchemaManager(): SchemaManager protected function getEntityMeta(): EntityMeta { - return self::$entityMeta ?: self::$entityMeta = new EntityMeta(new Psr16Cache(new ArrayAdapter())); + return self::$entityMeta instanceof EntityMeta ? self::$entityMeta : self::$entityMeta = new EntityMeta(new Psr16Cache(new ArrayAdapter())); } protected function flushDBCache(): void diff --git a/tests/FieldMapper/TestInvalidModel.php b/tests/FieldMapper/TestInvalidModel.php new file mode 100644 index 0000000..6d3999a --- /dev/null +++ b/tests/FieldMapper/TestInvalidModel.php @@ -0,0 +1,11 @@ +myId; + } + + public function setMyId(string $myId): void + { + $this->myId = $myId; + } + + public function getRelations(): ?Collection + { + return $this->relations; + } + + public function setRelations(?Collection $relations): void + { + $this->relations = $relations; + } +} diff --git a/tests/FieldMapper/TestParent.php b/tests/FieldMapper/TestParent.php index 7ebf34c..b680bd3 100644 --- a/tests/FieldMapper/TestParent.php +++ b/tests/FieldMapper/TestParent.php @@ -1,5 +1,7 @@ subUserId; + } + + public function setSubUserId(string $subUserId): void + { + $this->subUserId = $subUserId; + } +} diff --git a/tests/FieldMapper/TestUserModel.php b/tests/FieldMapper/TestUserModel.php new file mode 100644 index 0000000..8fe4582 --- /dev/null +++ b/tests/FieldMapper/TestUserModel.php @@ -0,0 +1,26 @@ +userId; + } + + public function setUserId(string $userId): void + { + $this->userId = $userId; + } +} diff --git a/tests/FieldMapperTest.php b/tests/FieldMapperTest.php index 01810cf..5847532 100644 --- a/tests/FieldMapperTest.php +++ b/tests/FieldMapperTest.php @@ -1,5 +1,7 @@ buildFrom(TestModel::class); - $expect = <<assertSame($expect, $actual); + } + + public function testInvalidEntity(): void + { + $this->expectException(OrmException::class); + + $connection = new MockConnection(); + $entityMeta = new EntityMeta(new Psr16Cache(new ArrayAdapter())); + $queryBuilder = new QueryBuilder($connection, $entityMeta); + + $queryBuilder->buildFrom(TestInvalidModel::class); + } + + public function testBlockedTableAlias(): void + { + $connection = new MockConnection(); + $entityMeta = new EntityMeta(new Psr16Cache(new ArrayAdapter())); + $queryBuilder = new QueryBuilder($connection, $entityMeta); + + $actual = $queryBuilder->buildFrom(TestSubTestUser::class); + $expect = 'FROM sub_user AS sub_user INNER JOIN user ON user_id = sub_user_id'; + + $this->assertSame($expect, $actual); + } + + public function testTooLongRelationName(): void + { + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage('The relation table name must be not larger than 30 characters'); + + $connection = new MockConnection(); + $entityMeta = new EntityMeta(new Psr16Cache(new ArrayAdapter())); + $queryBuilder = new QueryBuilder($connection, $entityMeta); - $this->assertEquals($expect, $actual); + $queryBuilder->buildFrom(TestModelWithTooLongRelation::class); } } From db6ead2ca7eb198327b003cdc65aef0c42614392 Mon Sep 17 00:00:00 2001 From: Marc Reichel Date: Thu, 28 Aug 2025 22:36:39 +0200 Subject: [PATCH 2/2] refactor: Simplify property declaration in `OrderBy` constructor --- src/OrderBy.php | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/src/OrderBy.php b/src/OrderBy.php index 8ec9970..18aefe0 100644 --- a/src/OrderBy.php +++ b/src/OrderBy.php @@ -12,13 +12,11 @@ */ class OrderBy implements OrderByInterface { - private string $orderBy { - set => $this->orderBy = ' ' . $value . ' '; - } - - public function __construct(string $orderBy) - { - $this->orderBy = $orderBy; + public function __construct( + private string $orderBy { + set => $this->orderBy = ' ' . $value . ' '; + }, + ) { } public function setOrderBy(string $orderBy): void