diff --git a/AGENTS.md b/AGENTS.md index 0f36e85..0b4a254 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -54,10 +54,13 @@ Integration tests need no external DB — they use in-memory SQLite (the `SqliteIntegrationTest` guards — extend it for any change to OR/NOT/subquery handling, and run it. - `CompositeSpecification` composes with **AND**. For OR use `OrSpecification` or - `SpecificationBuilder::orWhere()`. + `SpecificationBuilder::orWhere()`. A callback that contains nested + `orWhere()`/`notWhere()` must return the nested builder. Query modifiers remain + outside OR branches. - `ComparisonSpecification` validates operators (whitelist) and value↔operator consistency (NULL only with `=,!=,<>,is,is not`; array ops require arrays; - `between` requires exactly two; LIKE ops require strings). + `between` requires a zero-based list of exactly two values; LIKE ops require + strings). - `ilike`/`not ilike` are `LIKE` with `caseSensitive: false` (`ILIKE` on PostgreSQL, plain `LIKE` elsewhere). `startsWith`/`endsWith`/`contains` carry a `LikeMatch` and hand a clean string to the builder, which escapes and adds diff --git a/CHANGELOG.md b/CHANGELOG.md index 478fe9b..45cc2b0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,21 @@ # Changelog +## Unreleased + +### Fixed + +- Reject unknown operators in direct `OrConditionSpecification` conditions + instead of allowing SQL syntax to be injected through the operator position. +- `SpecificationBuilder::orWhere()` now keeps `ORDER BY`, `LIMIT` and `OFFSET` + outside OR branches, and nested `orWhere()` / `notWhere()` callbacks retain + the builder returned by the callback. +- `BETWEEN` values must be zero-based lists with exactly two elements, so + malformed associative arrays fail during specification construction. +- `DateTimeInterface` values are passed through `yiisoft/db`'s timezone-aware + `DateTimeValue` with six-digit fractional precision. +- Update the test adapter dependency to `rasuvaeff/property-testing-testo` + `^0.10`. + ## 1.3.1 — 2026-09-13 ### Fixed diff --git a/README.md b/README.md index 662e997..2e59f03 100644 --- a/README.md +++ b/README.md @@ -228,6 +228,10 @@ composer install && php examples/builder.php - **Values are parameterized.** All comparison/IN/BETWEEN/LIKE values are bound as parameters by `yiisoft/db`, so they are safe against SQL injection. +- **OR condition operators are allow-listed.** Direct + `OrConditionSpecification` conditions accept only the same canonical + operators as `ComparisonSpecification`; an unknown operator throws + `InvalidArgumentException` before query generation. - **Column names are not validated** — they are passed to `yiisoft/db` and quoted as identifiers, but there is no allow-list. Pass only **trusted** column names (typically hard-coded), never raw user input. @@ -290,7 +294,16 @@ Benchmarks live in `benchmarks/` and run via `composer bench` (requires path: a `null` operand becomes `IS NULL` / `IS NOT NULL`, anything else a plain equality — never a verbatim `IS 'value'`, which MySQL and PostgreSQL reject. A condition handed to `OrConditionSpecification` directly is normalized the - same way only in its three-element `[operator, column, value]` form. + same way only in its three-element `[operator, column, value]` form. Its + list-form operator must be one of the allow-listed comparison operators. +- `orWhere()` and `notWhere()` callbacks may return the nested builder; doing so + is required when the callback itself contains another `orWhere()` or + `notWhere()`. `ORDER BY`, `LIMIT` and `OFFSET` are query-level modifiers and + apply to the complete OR expression, even when written before or inside the + callback. +- `DateTimeInterface` values are rendered through `yiisoft/db`'s + timezone-aware `DateTimeValue` with six fractional digits. The target column + should support the resulting timezone-aware datetime representation. ## License diff --git a/README.ru.md b/README.ru.md index b6f2808..cd412a2 100644 --- a/README.ru.md +++ b/README.ru.md @@ -229,6 +229,10 @@ composer install && php examples/builder.php - **Значения параметризованы.** Все значения в сравнениях/IN/BETWEEN/LIKE биндятся `yiisoft/db` как параметры, поэтому защищены от SQL-инъекций. +- **Операторы OR проверяются по allow-list.** В прямых условиях + `OrConditionSpecification` разрешены только те же канонические операторы, + что и у `ComparisonSpecification`; неизвестный оператор вызывает + `InvalidArgumentException` до построения запроса. - **Имена колонок не валидируются** — они передаются в `yiisoft/db` и квотируются как идентификаторы, но allow-list'а нет. Передавайте только **доверенные** имена колонок (как правило, захардкоженные), никогда — сырой @@ -296,7 +300,15 @@ $spec = CompositeSpecification::create() становится `IS NULL` / `IS NOT NULL`, любой другой — обычным равенством, и никогда — буквальным `IS 'value'`, которое MySQL и PostgreSQL отвергают. Условие, переданное в `OrConditionSpecification` напрямую, нормализуется так - же только в трёхэлементной форме `[operator, column, value]`. + же только в трёхэлементной форме `[operator, column, value]`. Оператор в + списковой форме обязан быть в allow-list операторов сравнения. +- Колбэки `orWhere()` и `notWhere()` могут возвращать вложенный билдер; это + обязательно, если внутри колбэка есть ещё один `orWhere()` или `notWhere()`. + `ORDER BY`, `LIMIT` и `OFFSET` — модификаторы всего запроса: они применяются + ко всему OR-выражению, даже если записаны до колбэка или внутри него. +- Значения `DateTimeInterface` проходят через timezone-aware + `DateTimeValue` из `yiisoft/db` с шестью знаками дробной секунды. Целевая + колонка должна поддерживать получаемое представление даты со timezone. ## Лицензия diff --git a/benchmarks/OrCompositionBench.php b/benchmarks/OrCompositionBench.php index 74f401b..ba01fb6 100644 --- a/benchmarks/OrCompositionBench.php +++ b/benchmarks/OrCompositionBench.php @@ -21,20 +21,11 @@ final class OrCompositionBench { #[Bench( callables: [ - 'direct' => [self::class, 'buildDirect'], + 'builder' => [self::class, 'buildViaOrWhere'], ], calls: 1_000, iterations: 10, )] - public static function buildViaOrWhere(): CompositeSpecification - { - return SpecificationBuilder::create() - ->whereEqual('status', 'active') - ->orWhere(fn($b) => $b->whereEqual('status', 'pending')) - ->orWhere(fn($b) => $b->whereEqual('status', 'trial')) - ->build(); - } - public static function buildDirect(): CompositeSpecification { return CompositeSpecification::create() @@ -47,4 +38,13 @@ public static function buildDirect(): CompositeSpecification ->withComparison(column: 'status', value: 'trial', operator: '='), )); } + + public static function buildViaOrWhere(): CompositeSpecification + { + return SpecificationBuilder::create() + ->whereEqual('status', 'active') + ->orWhere(static fn(SpecificationBuilder $builder): SpecificationBuilder => $builder->whereEqual('status', 'pending')) + ->orWhere(static fn(SpecificationBuilder $builder): SpecificationBuilder => $builder->whereEqual('status', 'trial')) + ->build(); + } } diff --git a/benchmarks/SpecificationBuilderBench.php b/benchmarks/SpecificationBuilderBench.php index a0f938d..f66efc0 100644 --- a/benchmarks/SpecificationBuilderBench.php +++ b/benchmarks/SpecificationBuilderBench.php @@ -16,11 +16,23 @@ final class SpecificationBuilderBench { #[Bench( callables: [ - 'direct' => [self::class, 'buildDirect'], + 'builder' => [self::class, 'buildViaBuilder'], ], calls: 1_000, iterations: 10, )] + public static function buildDirect(): CompositeSpecification + { + return CompositeSpecification::create() + ->withComparison(column: 'status', value: 'active', operator: '=') + ->withComparison(column: 'age', value: 18, operator: '>') + ->withComparison(column: 'role', value: ['admin', 'editor', 'viewer'], operator: 'in') + ->withComparison(column: 'email', value: '%@example.com', operator: 'like') + ->withComparison(column: 'verified_at', value: null, operator: 'is not') + ->withLimit(limit: 100) + ->withOffset(offset: 0); + } + public static function buildViaBuilder(): CompositeSpecification { return SpecificationBuilder::create() @@ -33,16 +45,4 @@ public static function buildViaBuilder(): CompositeSpecification ->offset(0) ->build(); } - - public static function buildDirect(): CompositeSpecification - { - return CompositeSpecification::create() - ->withComparison(column: 'status', value: 'active', operator: '=') - ->withComparison(column: 'age', value: 18, operator: '>') - ->withComparison(column: 'role', value: ['admin', 'editor', 'viewer'], operator: 'in') - ->withComparison(column: 'email', value: '%@example.com', operator: 'like') - ->withComparison(column: 'verified_at', value: null, operator: 'is not') - ->withLimit(limit: 100) - ->withOffset(offset: 0); - } } diff --git a/composer.json b/composer.json index a20aef0..7cf5d2e 100644 --- a/composer.json +++ b/composer.json @@ -37,7 +37,7 @@ "friendsofphp/php-cs-fixer": "^3.95", "infection/infection": "^0.33", "maglnet/composer-require-checker": "^4.17", - "rasuvaeff/property-testing-testo": "^0.4", + "rasuvaeff/property-testing-testo": "^0.10", "rasuvaeff/rector-named-literals": "^1.0", "rector/rector": "^2.4", "roave/backward-compatibility-check": "^8.0", diff --git a/llms.txt b/llms.txt index 56fad3b..c375fd6 100644 --- a/llms.txt +++ b/llms.txt @@ -111,8 +111,10 @@ in, notIn, between(col,from,to), notBetween(col,from,to), isNull, isNotNull ``` Valid operators: `= != <> > >= < <= like not like ilike not ilike in not in between not between is is not`. Validation: NULL only with `= != <> is is not`; -`in/not in/between/not between` require array (`between` exactly two); LIKE ops -require string. `DateTimeInterface` values are normalized to `Y-m-d H:i:s`. +`in/not in/between/not between` require array (`between` is a zero-based list of +exactly two values); LIKE ops require string. Direct list-form conditions in +`OrConditionSpecification` reject unknown operators. `DateTimeInterface` values +use yiisoft/db `DateTimeValue` with timezone and six fractional digits. ## Performance diff --git a/src/ComparisonSpecification.php b/src/ComparisonSpecification.php index bd8b4c0..e978dd2 100644 --- a/src/ComparisonSpecification.php +++ b/src/ComparisonSpecification.php @@ -81,7 +81,10 @@ private function validateValue(): void ); } - if (in_array(needle: $this->operator, haystack: ['between', 'not between'], strict: true) && count(value: $this->value) !== 2) { + if ( + in_array(needle: $this->operator, haystack: ['between', 'not between'], strict: true) + && (!array_is_list(array: $this->value) || count(value: $this->value) !== 2) + ) { throw new InvalidArgumentException( message: sprintf('Operator "%s" requires array with exactly two values', $this->operator), ); diff --git a/src/OrConditionSpecification.php b/src/OrConditionSpecification.php index 68e1dd8..af07850 100644 --- a/src/OrConditionSpecification.php +++ b/src/OrConditionSpecification.php @@ -4,6 +4,8 @@ namespace Rasuvaeff\Specification; +use InvalidArgumentException; + /** * @implements Specification * @api @@ -20,7 +22,21 @@ /** * @param list> $conditions */ - public function __construct(private array $conditions = []) {} + public function __construct(private array $conditions = []) + { + foreach ($conditions as $condition) { + if (!array_is_list(array: $condition) || $condition === [] || !is_string(value: $condition[0])) { + continue; + } + + $operator = mb_strtolower(string: $condition[0]); + if (!in_array(needle: $operator, haystack: self::VALID_OPERATORS, strict: true)) { + throw new InvalidArgumentException( + message: sprintf('Invalid OR condition operator "%s"', $condition[0]), + ); + } + } + } /** * @template TVisitorReturn diff --git a/src/QueryBuildingVisitor.php b/src/QueryBuildingVisitor.php index 27680cd..34ac4f9 100644 --- a/src/QueryBuildingVisitor.php +++ b/src/QueryBuildingVisitor.php @@ -4,7 +4,9 @@ namespace Rasuvaeff\Specification; +use Yiisoft\Db\Constant\ColumnType; use Yiisoft\Db\Expression\ExpressionInterface; +use Yiisoft\Db\Expression\Value\DateTimeValue; use Yiisoft\Db\Query\QueryInterface; use Yiisoft\Db\QueryBuilder\Condition\LikeMode; @@ -29,7 +31,7 @@ public function visitComparison(ComparisonSpecification $specification): void case 'between': case 'not between': $value = $specification->getValue(); - if (!is_array(value: $value) || count(value: $value) !== 2) { + if (!is_array(value: $value) || !array_is_list(array: $value) || count(value: $value) !== 2) { throw new \InvalidArgumentException( message: sprintf('%s operator requires array with exactly two values', strtoupper(string: $operator)), ); @@ -53,7 +55,7 @@ public function visitComparison(ComparisonSpecification $specification): void $this->query->andWhere([ $operator === 'is' ? '=' : '!=', $specification->getColumn(), - $specification->getValue(), + $this->normalizeValue(value: $specification->getValue()), ]); break; @@ -74,7 +76,13 @@ public function visitComparison(ComparisonSpecification $specification): void private function normalizeValue(mixed $value): mixed { if ($value instanceof \DateTimeInterface) { - return $value->format('Y-m-d H:i:s'); + return new DateTimeValue(value: $value, type: ColumnType::DATETIMETZ, info: ['size' => 6]); + } + + if (is_array(value: $value)) { + foreach ($value as $key => $nestedValue) { + $value[$key] = $this->normalizeValue(value: $nestedValue); + } } return $value; @@ -126,6 +134,16 @@ public function visitOr(OrSpecification $specification): void $visitor = new self($subQuery); $childSpecification->accept($visitor); + if ($subQuery->getOrderBy() !== []) { + $this->query->addOrderBy($subQuery->getOrderBy()); + } + if (($limit = $subQuery->getLimit()) !== null) { + $this->query->limit($limit); + } + if (($offset = $subQuery->getOffset()) !== null) { + $this->query->offset($offset); + } + $where = $subQuery->getWhere(); if ($where === null) { continue; @@ -173,23 +191,36 @@ public function visitOrCondition(OrConditionSpecification $specification): void private function normalizeOrCondition(array $condition): array { if (!array_is_list(array: $condition) || count(value: $condition) !== 3) { - return $condition; + return $this->normalizeCondition(condition: $condition); } [$operator, $column, $value] = $condition; if (!is_string(value: $operator) || !is_string(value: $column)) { - return $condition; + return $this->normalizeCondition(condition: $condition); } $operator = mb_strtolower(string: $operator); if ($operator === 'is' || $operator === 'is not') { - return [$operator === 'is' ? '=' : '!=', $column, $value]; + return [$operator === 'is' ? '=' : '!=', $column, $this->normalizeValue(value: $value)]; } if (!is_string(value: $value) || !in_array(needle: $operator, haystack: self::LIKE_OPERATORS, strict: true)) { - return $condition; + return $this->normalizeCondition(condition: $condition); } return $this->likeCondition($operator, $column, $value, LikeMatch::Pattern); } + /** + * @param array $condition + * @return array + */ + private function normalizeCondition(array $condition): array + { + foreach ($condition as $key => $value) { + $condition[$key] = $this->normalizeValue(value: $value); + } + + return $condition; + } + /** * yiisoft/db reads `mode`, `escape` and `caseSensitive` from named operands * only and defaults to escaping the value and wrapping it in `%…%`. A @@ -368,6 +399,9 @@ private function createSubQuery(): QueryInterface $subQuery = clone $this->query; $subQuery->setWhere(null); $subQuery->params([]); + $subQuery->orderBy([]); + $subQuery->limit(null); + $subQuery->offset(null); return $subQuery; } diff --git a/src/SpecificationBuilder.php b/src/SpecificationBuilder.php index fb4d81f..dbafe83 100644 --- a/src/SpecificationBuilder.php +++ b/src/SpecificationBuilder.php @@ -127,20 +127,54 @@ public function whereContains(string $column, string $substring): self public function orWhere(callable $callback): self { $builder = new self(mutable: true); - $callback($builder); + $callbackResult = $callback($builder); + if ($callbackResult instanceof self) { + $builder = $callbackResult; + } $newBuilder = clone $this; - $built = $builder->build(); - - $existingSpecifications = $this->specification->getSpecifications(); - $orSpecifications = [$this->specification, $built]; + [$existingConditions, $existingModifiers] = $this->splitModifiers(specification: $this->specification); + [$callbackConditions, $callbackModifiers] = $this->splitModifiers(specification: $builder->build()); + + /** @var list $orSpecifications */ + $orSpecifications = []; + $callbackBranch = $callbackConditions === [] + ? null + : new CompositeSpecification(specifications: $callbackConditions); + if ($existingConditions !== []) { + if ( + count($existingConditions) === 1 + && $existingConditions[0] instanceof OrSpecification + && $callbackBranch instanceof CompositeSpecification + ) { + foreach ($existingConditions[0]->getSpecifications() as $existingSpecification) { + $orSpecifications[] = $existingSpecification; + } + $orSpecifications[] = $callbackBranch; + } else { + $orSpecifications[] = new CompositeSpecification(specifications: $existingConditions); + if ($callbackBranch instanceof CompositeSpecification) { + $orSpecifications[] = $callbackBranch; + } + } + } elseif ($callbackBranch instanceof CompositeSpecification) { + $orSpecifications[] = $callbackBranch; + } - if (count($existingSpecifications) === 1 && $existingSpecifications[0] instanceof OrSpecification) { - $orSpecifications = [...$existingSpecifications[0]->getSpecifications(), $built]; + $newBuilder->specification = CompositeSpecification::create(); + if (count($orSpecifications) === 1) { + foreach ($orSpecifications as $singleSpecification) { + $newBuilder->specification = $newBuilder->specification->withSpecification(specification: $singleSpecification); + } + } elseif ($orSpecifications !== []) { + $newBuilder->specification = $newBuilder->specification->withSpecification( + specification: OrSpecification::create(...$orSpecifications), + ); } - $newBuilder->specification = CompositeSpecification::create() - ->withSpecification(specification: OrSpecification::create(...$orSpecifications)); + foreach ([...$existingModifiers, ...$callbackModifiers] as $modifier) { + $newBuilder->specification = $newBuilder->specification->withSpecification(specification: $modifier); + } return $newBuilder; } @@ -151,10 +185,19 @@ public function orWhere(callable $callback): self public function notWhere(callable $callback): self { $builder = new self(mutable: true); - $callback($builder); + $callbackResult = $callback($builder); + if ($callbackResult instanceof self) { + $builder = $callbackResult; + } $newBuilder = clone $this; - $newBuilder->specification = $newBuilder->specification->withNot(specification: $builder->build()); + [$callbackConditions, $callbackModifiers] = $this->splitModifiers(specification: $builder->build()); + $newBuilder->specification = $newBuilder->specification->withNot( + specification: new CompositeSpecification(specifications: $callbackConditions), + ); + foreach ($callbackModifiers as $modifier) { + $newBuilder->specification = $newBuilder->specification->withSpecification(specification: $modifier); + } return $newBuilder; } @@ -195,4 +238,27 @@ public static function create(): self { return new self(); } + + /** + * @return array{list, list} + */ + private function splitModifiers(CompositeSpecification $specification): array + { + $conditions = []; + $modifiers = []; + + foreach ($specification->getSpecifications() as $childSpecification) { + if ( + $childSpecification instanceof OrderBySpecification + || $childSpecification instanceof LimitSpecification + || $childSpecification instanceof OffsetSpecification + ) { + $modifiers[] = $childSpecification; + } else { + $conditions[] = $childSpecification; + } + } + + return [$conditions, $modifiers]; + } } diff --git a/tests/ComparisonSpecificationTest.php b/tests/ComparisonSpecificationTest.php index cfc2e73..19dc9d4 100644 --- a/tests/ComparisonSpecificationTest.php +++ b/tests/ComparisonSpecificationTest.php @@ -442,6 +442,13 @@ public function validationBetweenRequiresTwoValues(): void new ComparisonSpecification(column: 'age', value: [18], operator: 'between'); } + public function validationBetweenRequiresAList(): void + { + Expect::exception(InvalidArgumentException::class)->withMessageContaining('Operator "between" requires array with exactly two values'); + + new ComparisonSpecification(column: 'price', value: ['from' => 20, 'to' => 40], operator: 'between'); + } + public function validationStringOperatorsRequireString(): void { Expect::exception(InvalidArgumentException::class)->withMessageContaining('Operator "like" requires string value'); diff --git a/tests/Integration/SqliteIntegrationTest.php b/tests/Integration/SqliteIntegrationTest.php index bf7e4fd..8db597d 100644 --- a/tests/Integration/SqliteIntegrationTest.php +++ b/tests/Integration/SqliteIntegrationTest.php @@ -4,10 +4,13 @@ namespace Rasuvaeff\Specification\Tests\Integration; +use DateTimeImmutable; +use DateTimeZone; use Rasuvaeff\Specification\ComparisonSpecification; use Rasuvaeff\Specification\CompositeSpecification; use Rasuvaeff\Specification\LikeMatch; use Rasuvaeff\Specification\NotSpecification; +use Rasuvaeff\Specification\OrConditionSpecification; use Rasuvaeff\Specification\OrSpecification; use Rasuvaeff\Specification\QueryApplier; use Rasuvaeff\Specification\SpecificationBuilder; @@ -120,6 +123,91 @@ public function orWhereBuilderKeepsBothSides(): void Assert::same($this->ids($spec), [1, 2, 4, 5]); } + public function orWherePreservesPagination(): void + { + $spec = SpecificationBuilder::create() + ->whereEqual(column: 'status', value: 'active') + ->orderBy(columns: ['price' => 'DESC']) + ->limit(limit: 1) + ->offset(offset: 2) + ->orWhere(static fn(SpecificationBuilder $b): SpecificationBuilder => $b->whereEqual(column: 'status', value: 'inactive')) + ->build(); + + $query = (new Query($this->db))->select('id')->from('items'); + QueryApplier::apply(specification: $spec, query: $query); + + $ids = array_map($this->rowId(...), $query->all()); + Assert::same($ids, [3]); + } + + public function orWhereCallbackPreservesItsPagination(): void + { + $spec = SpecificationBuilder::create() + ->whereEqual(column: 'status', value: 'active') + ->orWhere(static fn(SpecificationBuilder $b): SpecificationBuilder => $b + ->whereEqual(column: 'status', value: 'inactive') + ->orderBy(columns: ['price' => 'DESC']) + ->limit(limit: 1) + ->offset(offset: 2)) + ->build(); + + $query = (new Query($this->db))->select('id')->from('items'); + QueryApplier::apply(specification: $spec, query: $query); + + $ids = array_map($this->rowId(...), $query->all()); + Assert::same($ids, [3]); + } + + public function nestedOrWhereKeepsNestedBranch(): void + { + $this->db->createCommand("INSERT INTO items (id, name, status, price, created_at) VALUES (6, 'foxtrot', 'pending', 60, '2024-06-01')")->execute(); + + $spec = SpecificationBuilder::create()->orWhere( + static fn(SpecificationBuilder $b): SpecificationBuilder => $b + ->whereEqual(column: 'status', value: 'active') + ->orWhere(static fn(SpecificationBuilder $nested): SpecificationBuilder => $nested->whereEqual(column: 'status', value: 'pending')), + )->build(); + + Assert::same($this->ids($spec), [1, 2, 4, 6]); + } + + public function nestedNotWhereKeepsNestedBranch(): void + { + $this->db->createCommand("INSERT INTO items (id, name, status, price, created_at) VALUES (6, 'foxtrot', 'pending', 60, '2024-06-01')")->execute(); + + $spec = SpecificationBuilder::create()->notWhere( + static fn(SpecificationBuilder $b): SpecificationBuilder => $b + ->whereEqual(column: 'status', value: 'active') + ->orWhere(static fn(SpecificationBuilder $nested): SpecificationBuilder => $nested->whereEqual(column: 'status', value: 'pending')), + )->build(); + + Assert::same($this->ids($spec), [3, 5]); + } + + public function dateTimeKeepsMicrosecondsAndTimezone(): void + { + $this->db->createCommand("INSERT INTO items (id, name, status, price, created_at) VALUES (6, 'foxtrot', 'active', 60, '2024-06-01 03:04:05.123456+03:00')")->execute(); + + $dateTime = new DateTimeImmutable('2024-06-01 03:04:05.123456', new DateTimeZone('+03:00')); + $spec = SpecificationBuilder::create()->whereEqual(column: 'created_at', value: $dateTime)->build(); + + Assert::same($this->ids($spec), [6]); + } + + public function orConditionDateTimeKeepsMicrosecondsAndTimezone(): void + { + $this->db->createCommand("INSERT INTO items (id, name, status, price, created_at) VALUES (6, 'foxtrot', 'active', 60, '2024-06-01 03:04:05.123456+03:00')")->execute(); + + $dateTime = new DateTimeImmutable('2024-06-01 03:04:05.123456', new DateTimeZone('+03:00')); + $spec = new CompositeSpecification(specifications: [ + new OrConditionSpecification(conditions: [ + ['=', 'created_at', $dateTime], + ]), + ]); + + Assert::same($this->ids($spec), [6]); + } + public function andCombinedWithOrManyParameters(): void { $spec = SpecificationBuilder::create() diff --git a/tests/OrConditionSpecificationTest.php b/tests/OrConditionSpecificationTest.php index 8cd33e2..1716fb7 100644 --- a/tests/OrConditionSpecificationTest.php +++ b/tests/OrConditionSpecificationTest.php @@ -4,9 +4,11 @@ namespace Rasuvaeff\Specification\Tests; +use InvalidArgumentException; use Rasuvaeff\Specification\OrConditionSpecification; use Testo\Assert; use Testo\Codecov\Covers; +use Testo\Expect; use Testo\Test; #[Test] @@ -89,6 +91,15 @@ public function directConstruction(): void Assert::equals($conditions[2], ['!=', 'type', 'spam']); } + public function directConstructionRejectsUnknownOperator(): void + { + Expect::exception(InvalidArgumentException::class)->withMessageContaining('Invalid OR condition operator'); + + new OrConditionSpecification(conditions: [ + ['= 1 OR 1=1 --', 'status', 'x'], + ]); + } + public function emptyConditions(): void { $spec = new OrConditionSpecification(conditions: []); diff --git a/tests/QueryBuildingVisitorTest.php b/tests/QueryBuildingVisitorTest.php index 02dc462..bfcb858 100644 --- a/tests/QueryBuildingVisitorTest.php +++ b/tests/QueryBuildingVisitorTest.php @@ -26,6 +26,8 @@ use Testo\Data\DataProvider; use Testo\Expect; use Testo\Test; +use Yiisoft\Db\Constant\ColumnType; +use Yiisoft\Db\Expression\Value\DateTimeValue; use Yiisoft\Db\Query\Query; use Yiisoft\Db\QueryBuilder\Condition\LikeMode; @@ -106,6 +108,20 @@ public function visitComparisonIsNotOperator(): void Assert::same($query->getWhere(), ['!=', 'deleted_at', null]); } + public function visitComparisonIsDateTimeValue(): void + { + $query = $this->makeQuery(); + $dateTime = new DateTimeImmutable('2024-01-02 03:04:05.123456+03:00'); + $spec = new ComparisonSpecification(column: 'created_at', value: $dateTime, operator: 'is'); + + $spec->accept(new QueryBuildingVisitor(query: $query)); + + $where = $query->getWhere(); + Assert::true(is_array($where)); + Assert::same($where[0], '='); + Assert::instanceOf($where[2], DateTimeValue::class); + } + public function visitComposite(): void { $query = $this->makeQuery(); @@ -502,14 +518,40 @@ public function visitComparisonDateTimeValue(): void { $query = $this->makeQuery(); $visitor = new QueryBuildingVisitor(query: $query); + $dateTime = new DateTimeImmutable('2024-01-02 03:04:05.123456+03:00'); $spec = ComparisonSpecification::greaterThan( column: 'created_at', - value: new DateTimeImmutable('2024-01-02 03:04:05'), + value: $dateTime, ); $visitor->visitComparison(specification: $spec); - Assert::same($query->getWhere(), ['>', 'created_at', '2024-01-02 03:04:05']); + $where = $query->getWhere(); + Assert::true(is_array($where)); + Assert::same($where[0], '>'); + Assert::same($where[1], 'created_at'); + Assert::instanceOf($where[2], DateTimeValue::class); + /** @var DateTimeValue $value */ + $value = $where[2]; + Assert::same($value->value, $dateTime); + Assert::same($value->type, ColumnType::DATETIMETZ); + Assert::same($value->info, ['size' => 6]); + } + + public function visitOrConditionDateTimeValue(): void + { + $query = $this->makeQuery(); + $dateTime = new DateTimeImmutable('2024-01-02 03:04:05.123456+03:00'); + $specification = new OrConditionSpecification([ + ['=', 'created_at', $dateTime], + ]); + + $specification->accept(new QueryBuildingVisitor(query: $query)); + + $where = $query->getWhere(); + Assert::true(is_array($where)); + Assert::same($where[0], 'or'); + Assert::instanceOf($where[1][2], DateTimeValue::class); } public function visitNotWithComparison(): void @@ -719,7 +761,12 @@ public function visitComparisonInWithDateTimeValues(): void $spec = new ComparisonSpecification(column: 'date_col', value: [$dt1, $dt2], operator: 'in'); $visitor->visitComparison(specification: $spec); - Assert::same($query->getWhere(), ['in', 'date_col', ['2024-01-01 00:00:00', '2024-06-01 00:00:00']]); + $where = $query->getWhere(); + Assert::true(is_array($where)); + Assert::same($where[0], 'in'); + Assert::same($where[1], 'date_col'); + Assert::instanceOf($where[2][0], DateTimeValue::class); + Assert::instanceOf($where[2][1], DateTimeValue::class); } public function visitOrderByWithLowercaseDirectionStrings(): void @@ -885,6 +932,41 @@ public static function betweenBuildsConditionWithBothBoundsGenerators(): array ]; } + /** + * @return iterable + */ + public static function betweenBuildsConditionWithBothBoundsExamples(): iterable + { + yield 'zero lower and upper bound' => ['price', 0, 0]; + yield 'negative bounds' => ['price', -100, -1]; + yield 'reversed bounds remain positional' => ['created_at', 20, 10]; + } + + #[Property(runs: 200)] + public function directOrConditionDateTimeIsNormalized(DateTimeImmutable $value): void + { + $query = $this->makeQuery(); + (new OrConditionSpecification(conditions: [ + ['=', 'created_at', $value], + ]))->accept(new QueryBuildingVisitor(query: $query)); + + $where = $query->getWhere(); + Assert::true(is_array($where)); + Assert::same($where[0], 'or'); + Assert::instanceOf($where[1][2], DateTimeValue::class); + /** @var DateTimeValue $dateTimeValue */ + $dateTimeValue = $where[1][2]; + Assert::same($dateTimeValue->value, $value); + Assert::same($dateTimeValue->type, ColumnType::DATETIMETZ); + Assert::same($dateTimeValue->info, ['size' => 6]); + } + + /** @return array */ + public static function directOrConditionDateTimeIsNormalizedGenerators(): array + { + return ['value' => Gen::datetime()]; + } + #[Property(runs: 300)] public function doubleNegationCollapsesToPlainCondition(string $column, int $value): void { diff --git a/tests/SpecificationBuilderTest.php b/tests/SpecificationBuilderTest.php index fa1257d..6da1ab4 100644 --- a/tests/SpecificationBuilderTest.php +++ b/tests/SpecificationBuilderTest.php @@ -296,6 +296,52 @@ public function orWhere(): void Assert::count($orRight->getSpecifications(), 2); } + public function nestedOrWhereUsesReturnedBuilder(): void + { + $builder = SpecificationBuilder::create()->orWhere( + static fn(SpecificationBuilder $nested): SpecificationBuilder => $nested + ->whereEqual(column: 'status', value: 'active') + ->orWhere( + static fn(SpecificationBuilder $deep): SpecificationBuilder => $deep->whereEqual( + column: 'status', + value: 'pending', + ), + ), + ); + + $specifications = $builder->build()->getSpecifications(); + Assert::count($specifications, 1); + /** @var CompositeSpecification $branch */ + $branch = $specifications[0]; + Assert::instanceOf($branch, CompositeSpecification::class); + $nested = $branch->getSpecifications()[0]; + Assert::instanceOf($nested, OrSpecification::class); + /** @var OrSpecification $nested */ + Assert::count($nested->getSpecifications(), 2); + } + + public function nestedNotWhereUsesReturnedBuilder(): void + { + $builder = SpecificationBuilder::create()->notWhere( + static fn(SpecificationBuilder $nested): SpecificationBuilder => $nested + ->whereEqual(column: 'status', value: 'active') + ->orWhere( + static fn(SpecificationBuilder $deep): SpecificationBuilder => $deep->whereEqual( + column: 'status', + value: 'pending', + ), + ), + ); + + $specifications = $builder->build()->getSpecifications(); + Assert::count($specifications, 1); + Assert::instanceOf($specifications[0], NotSpecification::class); + /** @var NotSpecification $not */ + $not = $specifications[0]; + $nested = $not->getSpecification()->getSpecifications()[0]; + Assert::instanceOf($nested, OrSpecification::class); + } + public function notWhere(): void { $builder = SpecificationBuilder::create()