collection: use proper modifiers for filtering arguments

nextras · Apr 6, 2022 · 9cb5ec6 · 9cb5ec6
1 parent 649f035
commit 9cb5ec6
Show file tree

Hide file tree

Showing 14 changed files with 101 additions and 32 deletions.
diff --git a/src/Collection/Functions/BaseCompareFunction.php b/src/Collection/Functions/BaseCompareFunction.php
@@ -73,7 +73,7 @@ public function processQueryBuilderExpression(
 			$value = $args[1];
 		}
 
-		return $this->evaluateInDb($expression, $value);
+		return $this->evaluateInDb($expression, $value, $expression->dbalModifier ?? '%any');
 	}
 
 
@@ -86,6 +86,11 @@ abstract protected function evaluateInPhp($sourceValue, $targetValue): bool;
 
 	/**
 	 * @param mixed $value
+	 * @phpstan-param literal-string $modifier
 	 */
-	abstract protected function evaluateInDb(DbalExpressionResult $expression, $value): DbalExpressionResult;
+	abstract protected function evaluateInDb(
+		DbalExpressionResult $expression,
+		$value,
+		string $modifier
+	): DbalExpressionResult;
 }
diff --git a/src/Collection/Functions/CompareEqualsFunction.php b/src/Collection/Functions/CompareEqualsFunction.php
@@ -8,6 +8,7 @@
 use function array_combine;
 use function array_map;
 use function count;
+use function explode;
 use function in_array;
 use function is_array;
 
@@ -26,15 +27,20 @@ protected function evaluateInPhp($sourceValue, $targetValue): bool
 
 
 	/** @inheritDoc */
-	protected function evaluateInDb(DbalExpressionResult $expression, $value): DbalExpressionResult
+	protected function evaluateInDb(DbalExpressionResult $expression, $value, string $modifier): DbalExpressionResult
 	{
 		if (is_array($value)) {
 			if (count($value) > 0) {
+				// Multi-column primary key handling
 				// extract column names for multiOr simplification
 				// array{%column, array<string>}
 				$args = $expression->getExpansionArguments();
 				if (count($args) === 2 && $args[0] === '%column' && is_array($args[1])) {
-					$columns = $args[1];
+					$modifiers = explode(',', $modifier);
+					$columns = [];
+					foreach ($args[1] as $i => $column) {
+						$columns[] = $column . $modifiers[$i];
+					}
 					$value = array_map(function ($value) use ($columns): array {
 						/** @var array<string, string>|false $combined */
 						$combined = array_combine($columns, $value);
@@ -47,15 +53,18 @@ protected function evaluateInDb(DbalExpressionResult $expression, $value): DbalE
 					}, $value);
 					return $expression->withArgs('%multiOr', [$value]);
 				} else {
-					return $expression->append('IN %any', $value);
+					if ($modifier !== '%any') {
+						$modifier .= '[]';
+					}
+					return $expression->append("IN $modifier", $value);
 				}
 			} else {
 				return $expression->withArgs('1=0', []);
 			}
 		} elseif ($value === null) {
 			return $expression->append('IS NULL');
 		} else {
-			return $expression->append('= %any', $value);
+			return $expression->append("= $modifier", $value);
 		}
 	}
 }
diff --git a/src/Collection/Functions/CompareGreaterThanEqualsFunction.php b/src/Collection/Functions/CompareGreaterThanEqualsFunction.php
@@ -16,8 +16,8 @@ protected function evaluateInPhp($sourceValue, $targetValue): bool
 
 
 	/** @inheritDoc */
-	protected function evaluateInDb(DbalExpressionResult $expression, $value): DbalExpressionResult
+	protected function evaluateInDb(DbalExpressionResult $expression, $value, string $modifier): DbalExpressionResult
 	{
-		return $expression->append(">= %any", $value);
+		return $expression->append(">= $modifier", $value);
 	}
 }
diff --git a/src/Collection/Functions/CompareGreaterThanFunction.php b/src/Collection/Functions/CompareGreaterThanFunction.php
@@ -16,8 +16,8 @@ protected function evaluateInPhp($sourceValue, $targetValue): bool
 
 
 	/** @inheritDoc */
-	protected function evaluateInDb(DbalExpressionResult $expression, $value): DbalExpressionResult
+	protected function evaluateInDb(DbalExpressionResult $expression, $value, string $modifier): DbalExpressionResult
 	{
-		return $expression->append("> %any", $value);
+		return $expression->append("> $modifier", $value);
 	}
 }
diff --git a/src/Collection/Functions/CompareNotEqualsFunction.php b/src/Collection/Functions/CompareNotEqualsFunction.php
@@ -8,6 +8,7 @@
 use function array_combine;
 use function array_map;
 use function count;
+use function explode;
 use function in_array;
 use function is_array;
 
@@ -26,15 +27,20 @@ protected function evaluateInPhp($sourceValue, $targetValue): bool
 
 
 	/** @inheritDoc */
-	protected function evaluateInDb(DbalExpressionResult $expression, $value): DbalExpressionResult
+	protected function evaluateInDb(DbalExpressionResult $expression, $value, string $modifier): DbalExpressionResult
 	{
 		if (is_array($value)) {
 			if (count($value) > 0) {
+				// Multi-column primary key handling
 				// extract column names for multiOr simplification
 				// array{%column, array<string>}
 				$args = $expression->getExpansionArguments();
 				if (count($args) === 2 && $args[0] === '%column' && is_array($args[1])) {
-					$columns = $args[1];
+					$modifiers = explode(',', $modifier);
+					$columns = [];
+					foreach ($args[1] as $i => $column) {
+						$columns[] = $column . $modifiers[$i];
+					}
 					$value = array_map(function ($value) use ($columns): array {
 						/** @var array<string, string>|false $combined */
 						$combined = array_combine($columns, $value);
@@ -47,15 +53,18 @@ protected function evaluateInDb(DbalExpressionResult $expression, $value): DbalE
 					}, $value);
 					return $expression->withArgs('NOT (%multiOr)', [$value]);
 				} else {
-					return $expression->append('NOT IN %any', $value);
+					if ($modifier !== '%any') {
+						$modifier .= '[]';
+					}
+					return $expression->append("NOT IN $modifier", $value);
 				}
 			} else {
 				return $expression->withArgs('1=1', []);
 			}
 		} elseif ($value === null) {
 			return $expression->append('IS NOT NULL');
 		} else {
-			return $expression->append('!= %any', $value);
+			return $expression->append("!= $modifier", $value);
 		}
 	}
 }
diff --git a/src/Collection/Functions/CompareSmallerThanEqualsFunction.php b/src/Collection/Functions/CompareSmallerThanEqualsFunction.php
@@ -16,8 +16,8 @@ protected function evaluateInPhp($sourceValue, $targetValue): bool
 
 
 	/** @inheritDoc */
-	protected function evaluateInDb(DbalExpressionResult $expression, $value): DbalExpressionResult
+	protected function evaluateInDb(DbalExpressionResult $expression, $value, string $modifier): DbalExpressionResult
 	{
-		return $expression->append("<= %any", $value);
+		return $expression->append("<= $modifier", $value);
 	}
 }
diff --git a/src/Collection/Functions/CompareSmallerThanFunction.php b/src/Collection/Functions/CompareSmallerThanFunction.php
@@ -16,8 +16,8 @@ protected function evaluateInPhp($sourceValue, $targetValue): bool
 
 
 	/** @inheritDoc */
-	protected function evaluateInDb(DbalExpressionResult $expression, $value): DbalExpressionResult
+	protected function evaluateInDb(DbalExpressionResult $expression, $value, string $modifier): DbalExpressionResult
 	{
-		return $expression->append("< %any", $value);
+		return $expression->append("< $modifier", $value);
 	}
 }
diff --git a/src/Collection/Helpers/DbalExpressionResult.php b/src/Collection/Helpers/DbalExpressionResult.php
@@ -61,6 +61,13 @@ class DbalExpressionResult
 	 */
 	public $propertyMetadata;
 
+	/**
+	 * Dbal modifier for particular column. Null if expression is a general expression.
+	 * @var string|null
+	 * @phpstan-var literal-string|null
+	 */
+	public $dbalModifier;
+
 	/**
 	 * Value normalizer callback for proper matching backing property type.
 	 * @var callable|null
@@ -76,6 +83,7 @@ class DbalExpressionResult
 	 * @param array<array<mixed>> $groupBy
 	 * @phpstan-param list<mixed> $args
 	 * @param bool $isHavingClause
+	 * @phpstan-param literal-string $dbalModifier
 	 */
 	public function __construct(
 		string $expression,
@@ -85,7 +93,8 @@ public function __construct(
 		?IDbalAggregator $aggregator = null,
 		bool $isHavingClause = false,
 		?PropertyMetadata $propertyMetadata = null,
-		?callable $valueNormalizer = null
+		?callable $valueNormalizer = null,
+		?string $dbalModifier = null
 	)
 	{
 		$this->expression = $expression;
@@ -96,6 +105,7 @@ public function __construct(
 		$this->isHavingClause = $isHavingClause;
 		$this->propertyMetadata = $propertyMetadata;
 		$this->valueNormalizer = $valueNormalizer;
+		$this->dbalModifier = $dbalModifier;
 
 		if ($aggregator !== null && !$isHavingClause) {
 			throw new InvalidArgumentException('Dbal expression with aggregator is expected to be defined as HAVING clause.');

diff --git a/src/Collection/Helpers/DbalQueryBuilderHelper.php b/src/Collection/Helpers/DbalQueryBuilderHelper.php
@@ -328,12 +328,14 @@ private function processTokens(
 			throw new InvalidArgumentException("Property expression '$propertyExpression' does not fetch specific property.");
 		}
 
+		$modifier = '';
 		$column = $this->toColumnExpr(
 			$currentEntityMetadata,
 			$propertyMetadata,
 			$currentConventions,
 			$currentAlias,
-			$propertyPrefixTokens
+			$propertyPrefixTokens,
+			$modifier
 		);
 
 		if ($makeDistinct) {
@@ -352,7 +354,8 @@ private function processTokens(
 			$propertyMetadata,
 			function ($value) use ($propertyMetadata, $currentConventions) {
 				return $this->normalizeValue($value, $propertyMetadata, $currentConventions);
-			}
+			},
+			$modifier
 		);
 	}
 
@@ -464,17 +467,21 @@ private function toColumnExpr(
 		PropertyMetadata $propertyMetadata,
 		IConventions $conventions,
 		string $alias,
-		string $propertyPrefixTokens
+		string $propertyPrefixTokens,
+		string &$modifier
 	)
 	{
 		if ($propertyMetadata->isPrimary && $propertyMetadata->isVirtual) { // primary-proxy
 			$primaryKey = $entityMetadata->getPrimaryKey();
 			if (count($primaryKey) > 1) { // composite primary key
 				$pair = [];
+				$modifiers = [];
 				foreach ($primaryKey as $columnName) {
 					$columnName = $conventions->convertEntityToStorageKey($propertyPrefixTokens . $columnName);
 					$pair[] = "{$alias}.{$columnName}";
+					$modifiers[] = $conventions->getModifier($columnName);
 				}
+				$modifier = implode(',', $modifiers);
 				return $pair;
 			} else {
 				$propertyName = $primaryKey[0];
@@ -484,6 +491,7 @@ private function toColumnExpr(
 		}
 
 		$columnName = $conventions->convertEntityToStorageKey($propertyPrefixTokens . $propertyName);
+		$modifier = $conventions->getModifier($columnName);
 		$columnExpr = "{$alias}.{$columnName}";
 		return $columnExpr;
 	}

diff --git a/src/Mapper/Dbal/Conventions/Conventions.php b/src/Mapper/Dbal/Conventions/Conventions.php
@@ -68,7 +68,7 @@ class Conventions implements IConventions
 	 */
 	protected $mappings;
 
-	/** @var array<string, string> */
+	/** @var array<string, literal-string> */
 	protected $modifiers;
 
 	/**
@@ -309,6 +309,12 @@ public function setModifier(string $storageKey, string $saveModifier): IConventi
 	}
 
 
+	public function getModifier(string $storageKey): ?string
+	{
+		return $this->modifiers[$storageKey] ?? null;
+	}
+
+
 	/**
 	 * @phpstan-return array{string,string}
 	 */

diff --git a/src/Mapper/Dbal/Conventions/IConventions.php b/src/Mapper/Dbal/Conventions/IConventions.php
@@ -99,7 +99,16 @@ public function setMapping(
 
 	/**
 	 * Sets column modifier for data transformation to Nextras Dbal layer.
+	 * @phpstan-param literal-string $saveModifier
 	 * @return static
 	 */
 	public function setModifier(string $storageKey, string $saveModifier): IConventions;
+
+
+	/**
+	 * Returns column's modifier for Nextras Dbal layer.
+	 * @return string|null
+	 * @phpstan-return literal-string|null
+	 */
+	public function getModifier(string $storageKey): ?string;
 }
diff --git a/tests/cases/integration/Collection/collection.where.phpt b/tests/cases/integration/Collection/collection.where.phpt
@@ -33,22 +33,19 @@ class CollectionWhereTest extends DataTestCase
 		])->fetchAll();
 		Assert::count(2, $all);
 
-
 		$all = $this->orm->authors->findBy([
 			ICollection::OR, // operator is irrelevant
 			'name' => ['Writer 1', 'Writer 2'],
 		])->fetchAll();
 		Assert::count(2, $all);
 
-
 		$all = $this->orm->books->findBy([
 			ICollection::AND,
 			'author' => 1,
 			'nextPart' => null,
 		])->fetchAll();
 		Assert::count(2, $all);
 
-
 		$all = $this->orm->books->findBy([
 			ICollection::AND,
 			'author' => 1,
@@ -57,7 +54,6 @@ class CollectionWhereTest extends DataTestCase
 		])->fetchAll();
 		Assert::count(1, $all);
 
-
 		$all = $this->orm->tags->findBy([
 			ICollection::OR,
 			[ICollection::AND, 'name' => 'Tag 1', 'isGlobal' => true], // match
@@ -66,7 +62,6 @@ class CollectionWhereTest extends DataTestCase
 		])->fetchAll();
 		Assert::count(2, $all);
 
-
 		$all = $this->orm->books->findBy([
 			ICollection::AND, // match 2
 			[ICollection::OR, 'title' => 'Book 1', 'author' => 1], // match 1, 2
@@ -147,6 +142,21 @@ class CollectionWhereTest extends DataTestCase
 	}
 
 
+	public function testFilterByLocalDateTime(): void
+	{
+		if ($this->section === Helper::SECTION_MSSQL) {
+			Environment::skip('MSSQL does not handle timezones as we need. Maybe we should investigate more this test.');
+		}
+
+		$books = $this->orm->books->findBy([
+			'publishedAt' => new DateTimeImmutable('2021-12-14 21:10:04'),
+		]);
+
+		Assert::equal(1, $books->countStored());
+		Assert::equal(1, $books->count());
+	}
+
+
 	private function moveToDifferentZone(DateTimeImmutable $dateTime): DateTimeImmutable
 	{
 		return $dateTime->setTimezone(new DateTimeZone("UTC"));

diff --git a/tests/cases/unit/Mapper/Dbal/DbalValueOperatorFunctionTest.phpt b/tests/cases/unit/Mapper/Dbal/DbalValueOperatorFunctionTest.phpt
@@ -31,6 +31,7 @@ class DbalValueOperatorFunctionTest extends TestCase
 	public function testOperators(BaseCompareFunction $function, array $expected, array $expr): void
 	{
 		$expressionResult = new DbalExpressionResult('%column', ['books.id']);
+		$expressionResult->dbalModifier = '%i';
 
 		$helper = Mockery::mock(DbalQueryBuilderHelper::class);
 		$helper->shouldReceive('processPropertyExpr')->once()->andReturn($expressionResult);
@@ -51,10 +52,10 @@ class DbalValueOperatorFunctionTest extends TestCase
 	protected function operatorTestProvider(): array
 	{
 		return [
-			[new CompareEqualsFunction(), ['%column = %any', 'books.id', 1], ['id', 1]],
-			[new CompareNotEqualsFunction(), ['%column != %any', 'books.id', 1], ['id', 1]],
-			[new CompareEqualsFunction(), ['%column IN %any', 'books.id', [1, 2]], ['id', [1, 2]]],
-			[new CompareNotEqualsFunction(), ['%column NOT IN %any', 'books.id', [1, 2]], ['id', [1, 2]]],
+			[new CompareEqualsFunction(), ['%column = %i', 'books.id', 1], ['id', 1]],
+			[new CompareNotEqualsFunction(), ['%column != %i', 'books.id', 1], ['id', 1]],
+			[new CompareEqualsFunction(), ['%column IN %i[]', 'books.id', [1, 2]], ['id', [1, 2]]],
+			[new CompareNotEqualsFunction(), ['%column NOT IN %i[]', 'books.id', [1, 2]], ['id', [1, 2]]],
 			[new CompareNotEqualsFunction(), ['%column IS NOT NULL', 'books.id'], ['id', null]],
 		];
 	}

diff --git a/...NextrasTests/Orm/Integration/Collection/CollectionWhereTest_testFilterByLocalDateTime.sql b/...NextrasTests/Orm/Integration/Collection/CollectionWhereTest_testFilterByLocalDateTime.sql
@@ -0,0 +1,2 @@
+SELECT COUNT(*) AS count FROM (SELECT "books"."id" FROM "books" AS "books" WHERE (("books"."published_at" = '2021-12-14 21:10:04.000000'::timestamp))) temp;
+SELECT "books".* FROM "books" AS "books" WHERE (("books"."published_at" = '2021-12-14 21:10:04.000000'::timestamp));
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1,2 @@
		SELECT COUNT(*) AS count FROM (SELECT "books"."id" FROM "books" AS "books" WHERE (("books"."published_at" = '2021-12-14 21:10:04.000000'::timestamp))) temp;
		SELECT "books".* FROM "books" AS "books" WHERE (("books"."published_at" = '2021-12-14 21:10:04.000000'::timestamp));