From 3ea0a1340cf1fcf2fe8e787d04a9c0114e315b62 Mon Sep 17 00:00:00 2001 From: Jan Skrasek Date: Sun, 9 Feb 2020 23:43:57 +0100 Subject: [PATCH 1/6] collection: undefined property in STI does not return null instead undefined value reference --- .../Functions/ValueOperatorFunction.php | 4 --- .../Helpers/ArrayCollectionHelper.php | 31 +++++++++++-------- .../Repository/repository.sti.phpt | 4 +++ tests/inc/model/contents/Thread.php | 4 +-- 4 files changed, 24 insertions(+), 19 deletions(-) diff --git a/src/Collection/Functions/ValueOperatorFunction.php b/src/Collection/Functions/ValueOperatorFunction.php index b5e4faca..ae74598a 100644 --- a/src/Collection/Functions/ValueOperatorFunction.php +++ b/src/Collection/Functions/ValueOperatorFunction.php @@ -24,10 +24,6 @@ public function processArrayExpression(ArrayCollectionHelper $helper, IEntity $e assert(count($args) === 3); $operator = $args[0]; $valueReference = $helper->getValue($entity, $args[1]); - if ($valueReference === null) { - return false; - } - if ($valueReference->propertyMetadata !== null) { $targetValue = $helper->normalizeValue($args[2], $valueReference->propertyMetadata, true); } else { diff --git a/src/Collection/Helpers/ArrayCollectionHelper.php b/src/Collection/Helpers/ArrayCollectionHelper.php index 06b5219f..95c3d6a1 100644 --- a/src/Collection/Helpers/ArrayCollectionHelper.php +++ b/src/Collection/Helpers/ArrayCollectionHelper.php @@ -84,13 +84,8 @@ public function createSorter(array $expressions): Closure $_b = $expression[0]->processArrayExpression($this, $b, $expression[2]); } else { \assert($expression[2] instanceof EntityMetadata); - $a_ref = $this->getValueByTokens($a, $expression[0], $expression[2]); - $b_ref = $this->getValueByTokens($b, $expression[0], $expression[2]); - if ($a_ref === null || $b_ref === null) { - throw new InvalidStateException('Comparing entities that should not be included in the result. Possible missing filtering configuration for required entity type based on Single Table Inheritance.'); - } - $_a = $a_ref->value; - $_b = $b_ref->value; + $_a = $this->getValueByTokens($a, $expression[0], $expression[2])->value; + $_b = $this->getValueByTokens($b, $expression[0], $expression[2])->value; } $ordering = $expression[1]; @@ -100,10 +95,8 @@ public function createSorter(array $expressions): Closure // By default, <=> sorts nulls at the beginning. $nullsReverse = $ordering === ICollection::ASC_NULLS_FIRST || $ordering === ICollection::DESC_NULLS_FIRST ? 1 : -1; $result = ($_a <=> $_b) * $nullsReverse; - } elseif (is_int($_a) || is_float($_a) || is_int($_b) || is_float($_b)) { $result = ($_a <=> $_b) * $descReverse; - } else { $result = ((string) $_a <=> (string) $_b) * $descReverse; } @@ -119,10 +112,9 @@ public function createSorter(array $expressions): Closure /** - * Returns value reference, returns null when entity should not be evaluated at all because of STI condition. * @param string|array $expr */ - public function getValue(IEntity $entity, $expr): ?ArrayPropertyValueReference + public function getValue(IEntity $entity, $expr): ArrayPropertyValueReference { if (is_array($expr)) { $function = array_shift($expr); @@ -188,10 +180,23 @@ public function normalizeValue($value, PropertyMetadata $propertyMetadata, bool /** * @param string[] $expressionTokens */ - private function getValueByTokens(IEntity $entity, array $expressionTokens, EntityMetadata $sourceEntityMeta): ?ArrayPropertyValueReference + private function getValueByTokens( + IEntity $entity, + array $expressionTokens, + EntityMetadata $sourceEntityMeta + ): ArrayPropertyValueReference { if (!$entity instanceof $sourceEntityMeta->className) { - return null; + return new ArrayPropertyValueReference( + new class { + public function __toString() + { + return "undefined"; + } + }, + false, + null + ); } $isMultiValue = false; diff --git a/tests/cases/integration/Repository/repository.sti.phpt b/tests/cases/integration/Repository/repository.sti.phpt index 8ba2b935..ee2590f3 100644 --- a/tests/cases/integration/Repository/repository.sti.phpt +++ b/tests/cases/integration/Repository/repository.sti.phpt @@ -23,6 +23,10 @@ class RepositorySTITest extends DataTestCase { $thread = $this->orm->contents->findBy(['id' => 1])->fetch(); Assert::type(Thread::class, $thread); + + foreach ($thread->comments->get() as $comment) { + Assert::type(Comment::class, $comment); + } } diff --git a/tests/inc/model/contents/Thread.php b/tests/inc/model/contents/Thread.php index 9ae65c58..f4157bcd 100644 --- a/tests/inc/model/contents/Thread.php +++ b/tests/inc/model/contents/Thread.php @@ -8,12 +8,12 @@ namespace NextrasTests\Orm; -use Nextras\Orm\Relationships\ManyHasOne; +use Nextras\Orm\Relationships\OneHasMany; /** * @property-read string $type {default thread} - * @property ManyHasOne|Comment[] $comments {1:m Comment::$thread, cascade=[persist, remove]} + * @property OneHasMany|Comment[] $comments {1:m Comment::$thread, cascade=[persist, remove]} */ class Thread extends ThreadCommentCommon { From 6dbb466d88a01a9b5e12af60db5544acdf55453a Mon Sep 17 00:00:00 2001 From: Jan Skrasek Date: Sun, 9 Feb 2020 18:47:25 +0100 Subject: [PATCH 2/6] collection: implement proper distinct (grouping) & reimplement for SQLSRV --- .../Helpers/DbalQueryBuilderHelper.php | 26 ++++++++++++++----- .../Mapper/Dbal/QueryBuilderHelperTest.phpt | 8 +++--- 2 files changed, 23 insertions(+), 11 deletions(-) diff --git a/src/Collection/Helpers/DbalQueryBuilderHelper.php b/src/Collection/Helpers/DbalQueryBuilderHelper.php index 2031a4e2..5070aae6 100644 --- a/src/Collection/Helpers/DbalQueryBuilderHelper.php +++ b/src/Collection/Helpers/DbalQueryBuilderHelper.php @@ -198,6 +198,7 @@ private function processTokens(array $tokens, ?string $sourceEntity, QueryBuilde $currentReflection = $currentMapper->getConventions(); $currentEntityMetadata = $currentMapper->getRepository()->getEntityMetadata($sourceEntity); $propertyPrefixTokens = ""; + $makeDistinct = false; foreach ($tokens as $tokenIndex => $token) { $property = $currentEntityMetadata->getProperty($token); @@ -206,7 +207,7 @@ private function processTokens(array $tokens, ?string $sourceEntity, QueryBuilde $currentAlias, $currentReflection, $currentEntityMetadata, - $currentMapper, + $currentMapper ] = $this->processRelationship( $tokens, $builder, @@ -215,7 +216,8 @@ private function processTokens(array $tokens, ?string $sourceEntity, QueryBuilde $currentMapper, $currentAlias, $token, - $tokenIndex + $tokenIndex, + $makeDistinct ); } elseif ($property->wrapper === EmbeddableContainer::class) { \assert($property->args !== null); @@ -226,6 +228,10 @@ private function processTokens(array $tokens, ?string $sourceEntity, QueryBuilde } } + if ($makeDistinct) { + $this->makeDistinct($builder, $this->mapper); + } + $propertyMetadata = $currentEntityMetadata->getProperty($lastToken); if ($propertyMetadata->wrapper === EmbeddableContainer::class) { $propertyExpression = \implode('->', \array_merge($tokens, [$lastToken])); @@ -264,7 +270,8 @@ private function processRelationship( DbalMapper $currentMapper, string $currentAlias, $token, - int $tokenIndex + int $tokenIndex, + bool & $makeDistinct ): array { \assert($property->relationship !== null); @@ -279,7 +286,7 @@ private function processRelationship( \assert($property->relationship->property !== null); $toColumn = $targetReflection->convertEntityToStorageKey($property->relationship->property); $fromColumn = $currentReflection->getStoragePrimaryKey()[0]; - $this->makeDistinct($builder); + $makeDistinct = true; } elseif ($relType === Relationship::ONE_HAS_ONE && !$property->relationship->isMain) { \assert($property->relationship->property !== null); @@ -289,7 +296,7 @@ private function processRelationship( } elseif ($relType === Relationship::MANY_HAS_MANY) { $toColumn = $targetReflection->getStoragePrimaryKey()[0]; $fromColumn = $currentReflection->getStoragePrimaryKey()[0]; - $this->makeDistinct($builder); + $makeDistinct = true; if ($property->relationship->isMain) { \assert($currentMapper instanceof DbalMapper); @@ -360,11 +367,16 @@ private function toColumnExpr( } - private function makeDistinct(QueryBuilder $builder) + private function makeDistinct(QueryBuilder $builder, DbalMapper $mapper) { $baseTable = $builder->getFromAlias(); if ($this->platformName === 'mssql') { - $builder->select('DISTINCT %table.*', $baseTable); + $tableName = $mapper->getTableName(); + $columns = $mapper->getDatabasePlatform()->getColumns($tableName); + $columnNames = \array_map(function ($column) use ($tableName) { + return $tableName . '.'. $column['name']; + }, $columns); + $builder->groupBy('%column[]', $columnNames); } else { $primaryKey = $this->mapper->getConventions()->getStoragePrimaryKey(); diff --git a/tests/cases/unit/Mapper/Dbal/QueryBuilderHelperTest.phpt b/tests/cases/unit/Mapper/Dbal/QueryBuilderHelperTest.phpt index 32cd824f..471a07b0 100644 --- a/tests/cases/unit/Mapper/Dbal/QueryBuilderHelperTest.phpt +++ b/tests/cases/unit/Mapper/Dbal/QueryBuilderHelperTest.phpt @@ -150,22 +150,22 @@ class QueryBuilderHelperTest extends TestCase $repository->shouldReceive('getMapper')->once()->andReturn($this->mapper); $this->mapper->shouldReceive('getConventions')->once()->andReturn($this->conventions); $this->mapper->shouldReceive('getManyHasManyParameters')->once()->with($propertyMetadata2, $this->mapper)->andReturn(['books_x_tags', ['book_id', 'tag_id']]); - $this->conventions->shouldReceive('getStoragePrimaryKey')->twice()->andReturn(['id']); + $this->conventions->shouldReceive('getStoragePrimaryKey')->once()->andReturn(['id']); $this->mapper->shouldReceive('getTableName')->once()->andReturn('tags'); // name $namePropertyMetadata = new PropertyMetadata(); $namePropertyMetadata->name = 'name'; $this->entityMetadata->shouldReceive('getProperty')->once()->with('name')->andReturn($namePropertyMetadata); - $this->mapper->shouldReceive('getConventions')->twice()->andReturn($this->conventions); + $this->mapper->shouldReceive('getConventions')->once()->andReturn($this->conventions); $this->conventions->shouldReceive('convertEntityToStorageKey')->once()->with('name')->andReturn('name'); $this->conventions->shouldReceive('getStoragePrimaryKey')->twice()->andReturn(['id']); $this->queryBuilder->shouldReceive('leftJoin')->once()->with('authors', '[books]', 'translatedBooks', '[authors.id] = [translatedBooks.translator_id]'); $this->queryBuilder->shouldReceive('leftJoin')->once()->with('translatedBooks', '[books_x_tags]', 'books_x_tags', '[translatedBooks.id] = [books_x_tags.book_id]'); $this->queryBuilder->shouldReceive('leftJoin')->once()->with('books_x_tags', '[tags]', 'translatedBooks_tags', '[books_x_tags.tag_id] = [translatedBooks_tags.id]'); - $this->queryBuilder->shouldReceive('getFromAlias')->twice()->andReturn('authors'); - $this->queryBuilder->shouldReceive('groupBy')->twice()->with('%column[]', ['authors.id']); + $this->queryBuilder->shouldReceive('getFromAlias')->once()->andReturn('authors'); + $this->queryBuilder->shouldReceive('groupBy')->once()->with('%column[]', ['authors.id']); $columnReference = $this->builderHelper->processPropertyExpr($this->queryBuilder, 'translatedBooks->tags->name'); Assert::same('translatedBooks_tags.name', $columnReference->args[1]); From f8652f261db9a5c7cc18482223eb640a49447748 Mon Sep 17 00:00:00 2001 From: Jan Skrasek Date: Sun, 9 Feb 2020 13:12:05 +0100 Subject: [PATCH 3/6] collection: implement aggregation functions --- .../Functions/AvgAggregateFunction.php | 24 ++++ .../Functions/BaseAggregateFunction.php | 66 +++++++++ .../Functions/CountAggregateFunction.php | 24 ++++ .../Functions/MaxAggregateFunction.php | 24 ++++ .../Functions/MinAggregateFunction.php | 24 ++++ .../Functions/SumAggregateFunction.php | 24 ++++ src/Repository/Repository.php | 28 ++-- .../Collection/collection.aggregation.phpt | 128 ++++++++++++++++++ 8 files changed, 334 insertions(+), 8 deletions(-) create mode 100644 src/Collection/Functions/AvgAggregateFunction.php create mode 100644 src/Collection/Functions/BaseAggregateFunction.php create mode 100644 src/Collection/Functions/CountAggregateFunction.php create mode 100644 src/Collection/Functions/MaxAggregateFunction.php create mode 100644 src/Collection/Functions/MinAggregateFunction.php create mode 100644 src/Collection/Functions/SumAggregateFunction.php create mode 100644 tests/cases/integration/Collection/collection.aggregation.phpt diff --git a/src/Collection/Functions/AvgAggregateFunction.php b/src/Collection/Functions/AvgAggregateFunction.php new file mode 100644 index 00000000..7bad96c3 --- /dev/null +++ b/src/Collection/Functions/AvgAggregateFunction.php @@ -0,0 +1,24 @@ +sqlFunction = $sqlFunction; + } + + + /** + * @param array $values + * @return number + */ + abstract protected function calculateAggregation(array $values); + + + public function processArrayExpression(ArrayCollectionHelper $helper, IEntity $entity, array $args) + { + \assert(\count($args) === 1 && \is_string($args[0])); + + $valueReference = $helper->getValue($entity, $args[0]); + if (!$valueReference->isMultiValue) { + throw new InvalidArgumentException('Aggregation is not called over has many relationship.'); + } + \assert(\is_array($valueReference->value)); + + return $this->calculateAggregation($valueReference->value); + } + + + public function processQueryBuilderExpression( + DbalQueryBuilderHelper $helper, + QueryBuilder $builder, + array $args + ): DbalExpressionResult + { + \assert(\count($args) === 1 && \is_string($args[0])); + + $expression = $helper->processPropertyExpr($builder, $args[0]); + return new DbalExpressionResult( + ["{$this->sqlFunction}(%ex)", $expression->args], + true + ); + } +} diff --git a/src/Collection/Functions/CountAggregateFunction.php b/src/Collection/Functions/CountAggregateFunction.php new file mode 100644 index 00000000..56870727 --- /dev/null +++ b/src/Collection/Functions/CountAggregateFunction.php @@ -0,0 +1,24 @@ + true, + ConjunctionOperatorFunction::class => true, + DisjunctionOperatorFunction::class => true, + AvgAggregateFunction::class => true, + CountAggregateFunction::class => true, + MaxAggregateFunction::class => true, + MinAggregateFunction::class => true, + SumAggregateFunction::class => true, + ]; + + if (isset($knownFunctions[$name])) { + return new $name(); } else { throw new NotImplementedException('Override ' . get_class($this) . '::createCollectionFunction() to return an instance of ' . $name . ' collection function.'); } diff --git a/tests/cases/integration/Collection/collection.aggregation.phpt b/tests/cases/integration/Collection/collection.aggregation.phpt new file mode 100644 index 00000000..62f2dcfa --- /dev/null +++ b/tests/cases/integration/Collection/collection.aggregation.phpt @@ -0,0 +1,128 @@ +orm->authors + ->findBy([ + ValueOperatorFunction::class, + '<', + [AvgAggregateFunction::class, 'books->price->cents'], + 110 + ]) + ->orderBy('id') + ->fetchPairs(null, 'id'); + Assert::same([1], $booksId); + + $booksId = $this->orm->authors + ->findBy([ + ValueOperatorFunction::class, + '<=', + [AvgAggregateFunction::class, 'books->price->cents'], + 120 + ]) + ->orderBy('id') + ->fetchPairs(null, 'id'); + Assert::same([1, 2], $booksId); + } + + public function testCount() + { + $bookIds = $this->orm->books + ->findAll() + ->findBy([ + ValueOperatorFunction::class, + '>=', + [CountAggregateFunction::class, 'tags->id'], + 2, + ]) + ->orderBy('id') + ->fetchPairs(null, 'id'); + Assert::same([1, 2], $bookIds); + + $bookIds = $this->orm->books + ->findAll() + ->orderBy([CountAggregateFunction::class, 'tags->id']) + ->orderBy('id') + ->fetchPairs(null, 'id'); + Assert::same([4, 3, 1, 2], $bookIds); + + $bookIds = $this->orm->books + ->findAll() + ->orderBy([CountAggregateFunction::class, 'tags->id'], ICollection::DESC) + ->orderBy('id', ICollection::DESC) + ->fetchPairs(null, 'id'); + Assert::same([2, 1, 3, 4], $bookIds); + } + + + public function testMax() + { + $userIds = $this->orm->authors + ->findBy([ + ValueOperatorFunction::class, + '>', + [MaxAggregateFunction::class, 'books->price->cents'], + 150 + ]) + ->orderBy('id') + ->fetchPairs(null, 'id'); + Assert::same([2], $userIds); + } + + + public function testMin() + { + $userIds = $this->orm->authors + ->findBy([ + ValueOperatorFunction::class, + '<', + [MinAggregateFunction::class, 'books->price->cents'], + 50 + ]) + ->orderBy('id') + ->fetchPairs(null, 'id'); + Assert::same([2], $userIds); + } + + + public function testSum() + { + $userIds = $this->orm->authors + ->findBy([ + ValueOperatorFunction::class, + '<=', + [SumAggregateFunction::class, 'books->price->cents'], + 200 + ]) + ->orderBy('id') + ->fetchPairs(null, 'id'); + Assert::same([1], $userIds); + } +} + + +$test = new CollectionAggregationTest($dic); +$test->run(); From 0cac9e6ab4162e262dacb367d5d8164cd39665b3 Mon Sep 17 00:00:00 2001 From: Jan Skrasek Date: Sun, 16 Feb 2020 14:49:28 +0100 Subject: [PATCH 4/6] collection: rename ValueOperatorFunction to CompareFunction --- ...lueOperatorFunction.php => CompareFunction.php} | 2 +- .../Functions/ConjunctionOperatorFunction.php | 2 +- .../Functions/DisjunctionOperatorFunction.php | 2 +- src/Repository/Repository.php | 4 ++-- .../Collection/collection.aggregation.phpt | 14 +++++++------- .../Mapper/Dbal/DbalValueOperatorFunctionTest.phpt | 4 ++-- 6 files changed, 14 insertions(+), 14 deletions(-) rename src/Collection/Functions/{ValueOperatorFunction.php => CompareFunction.php} (98%) diff --git a/src/Collection/Functions/ValueOperatorFunction.php b/src/Collection/Functions/CompareFunction.php similarity index 98% rename from src/Collection/Functions/ValueOperatorFunction.php rename to src/Collection/Functions/CompareFunction.php index ae74598a..9ef70398 100644 --- a/src/Collection/Functions/ValueOperatorFunction.php +++ b/src/Collection/Functions/CompareFunction.php @@ -17,7 +17,7 @@ use Nextras\Orm\InvalidArgumentException; -class ValueOperatorFunction implements IArrayFunction, IQueryBuilderFunction +class CompareFunction implements IArrayFunction, IQueryBuilderFunction { public function processArrayExpression(ArrayCollectionHelper $helper, IEntity $entity, array $args) { diff --git a/src/Collection/Functions/ConjunctionOperatorFunction.php b/src/Collection/Functions/ConjunctionOperatorFunction.php index 14e6a3c8..355aca5d 100644 --- a/src/Collection/Functions/ConjunctionOperatorFunction.php +++ b/src/Collection/Functions/ConjunctionOperatorFunction.php @@ -61,7 +61,7 @@ private function normalizeFunctions(array $args): array $processedArgs = []; foreach ($args as $argName => $argValue) { [$argName, $operator] = ConditionParserHelper::parsePropertyOperator($argName); - $processedArgs[] = [ValueOperatorFunction::class, $operator, $argName, $argValue]; + $processedArgs[] = [CompareFunction::class, $operator, $argName, $argValue]; } return $processedArgs; } diff --git a/src/Collection/Functions/DisjunctionOperatorFunction.php b/src/Collection/Functions/DisjunctionOperatorFunction.php index 9aedbf6c..67ebccb9 100644 --- a/src/Collection/Functions/DisjunctionOperatorFunction.php +++ b/src/Collection/Functions/DisjunctionOperatorFunction.php @@ -61,7 +61,7 @@ private function normalizeFunctions(array $args): array $processedArgs = []; foreach ($args as $argName => $argValue) { [$argName, $operator] = ConditionParserHelper::parsePropertyOperator($argName); - $processedArgs[] = [ValueOperatorFunction::class, $operator, $argName, $argValue]; + $processedArgs[] = [CompareFunction::class, $operator, $argName, $argValue]; } return $processedArgs; } diff --git a/src/Repository/Repository.php b/src/Repository/Repository.php index 3e70adfb..7ee1b604 100644 --- a/src/Repository/Repository.php +++ b/src/Repository/Repository.php @@ -16,7 +16,7 @@ use Nextras\Orm\Collection\Functions\SumAggregateFunction; use Nextras\Orm\Collection\Functions\ConjunctionOperatorFunction; use Nextras\Orm\Collection\Functions\DisjunctionOperatorFunction; -use Nextras\Orm\Collection\Functions\ValueOperatorFunction; +use Nextras\Orm\Collection\Functions\CompareFunction; use Nextras\Orm\Collection\ICollection; use Nextras\Orm\Entity\IEntity; use Nextras\Orm\Entity\Reflection\EntityMetadata; @@ -230,7 +230,7 @@ public function getCollectionFunction(string $name) protected function createCollectionFunction(string $name) { static $knownFunctions = [ - ValueOperatorFunction::class => true, + CompareFunction::class => true, ConjunctionOperatorFunction::class => true, DisjunctionOperatorFunction::class => true, AvgAggregateFunction::class => true, diff --git a/tests/cases/integration/Collection/collection.aggregation.phpt b/tests/cases/integration/Collection/collection.aggregation.phpt index 62f2dcfa..22f7b37b 100644 --- a/tests/cases/integration/Collection/collection.aggregation.phpt +++ b/tests/cases/integration/Collection/collection.aggregation.phpt @@ -8,11 +8,11 @@ namespace NextrasTests\Orm\Integration\Collection; use Nextras\Orm\Collection\Functions\AvgAggregateFunction; +use Nextras\Orm\Collection\Functions\CompareFunction; use Nextras\Orm\Collection\Functions\CountAggregateFunction; use Nextras\Orm\Collection\Functions\MaxAggregateFunction; use Nextras\Orm\Collection\Functions\MinAggregateFunction; use Nextras\Orm\Collection\Functions\SumAggregateFunction; -use Nextras\Orm\Collection\Functions\ValueOperatorFunction; use Nextras\Orm\Collection\ICollection; use NextrasTests\Orm\DataTestCase; use Tester\Assert; @@ -27,7 +27,7 @@ class CollectionAggregationTest extends DataTestCase { $booksId = $this->orm->authors ->findBy([ - ValueOperatorFunction::class, + CompareFunction::class, '<', [AvgAggregateFunction::class, 'books->price->cents'], 110 @@ -38,7 +38,7 @@ class CollectionAggregationTest extends DataTestCase $booksId = $this->orm->authors ->findBy([ - ValueOperatorFunction::class, + CompareFunction::class, '<=', [AvgAggregateFunction::class, 'books->price->cents'], 120 @@ -53,7 +53,7 @@ class CollectionAggregationTest extends DataTestCase $bookIds = $this->orm->books ->findAll() ->findBy([ - ValueOperatorFunction::class, + CompareFunction::class, '>=', [CountAggregateFunction::class, 'tags->id'], 2, @@ -82,7 +82,7 @@ class CollectionAggregationTest extends DataTestCase { $userIds = $this->orm->authors ->findBy([ - ValueOperatorFunction::class, + CompareFunction::class, '>', [MaxAggregateFunction::class, 'books->price->cents'], 150 @@ -97,7 +97,7 @@ class CollectionAggregationTest extends DataTestCase { $userIds = $this->orm->authors ->findBy([ - ValueOperatorFunction::class, + CompareFunction::class, '<', [MinAggregateFunction::class, 'books->price->cents'], 50 @@ -112,7 +112,7 @@ class CollectionAggregationTest extends DataTestCase { $userIds = $this->orm->authors ->findBy([ - ValueOperatorFunction::class, + CompareFunction::class, '<=', [SumAggregateFunction::class, 'books->price->cents'], 200 diff --git a/tests/cases/unit/Mapper/Dbal/DbalValueOperatorFunctionTest.phpt b/tests/cases/unit/Mapper/Dbal/DbalValueOperatorFunctionTest.phpt index 00b1f6fe..326d12be 100644 --- a/tests/cases/unit/Mapper/Dbal/DbalValueOperatorFunctionTest.phpt +++ b/tests/cases/unit/Mapper/Dbal/DbalValueOperatorFunctionTest.phpt @@ -8,7 +8,7 @@ namespace NextrasTests\Orm\Mapper\Dbal; use Mockery; use Nextras\Dbal\QueryBuilder\QueryBuilder; -use Nextras\Orm\Collection\Functions\ValueOperatorFunction; +use Nextras\Orm\Collection\Functions\CompareFunction; use Nextras\Orm\Collection\Helpers\ConditionParserHelper; use Nextras\Orm\Collection\Helpers\DbalExpressionResult; use Nextras\Orm\Collection\Helpers\DbalQueryBuilderHelper; @@ -26,7 +26,7 @@ class DbalValueOperatorFunctionTest extends TestCase */ public function testOperators($expected, $expr) { - $valueOperatorFunction = new ValueOperatorFunction(); + $valueOperatorFunction = new CompareFunction(); $expressionResult = new DbalExpressionResult(['%column', 'books.id']); From d2fca018a53dcf09de703d87222dda5995be2f41 Mon Sep 17 00:00:00 2001 From: Jan Skrasek Date: Sun, 16 Feb 2020 14:56:08 +0100 Subject: [PATCH 5/6] collection: move operator const to CompareFunction --- src/Collection/Functions/CompareFunction.php | 25 ++++++++++++------- .../Helpers/ConditionParserHelper.php | 13 +++------- .../Dbal/DbalValueOperatorFunctionTest.phpt | 11 ++++---- 3 files changed, 24 insertions(+), 25 deletions(-) diff --git a/src/Collection/Functions/CompareFunction.php b/src/Collection/Functions/CompareFunction.php index 9ef70398..487cc4b4 100644 --- a/src/Collection/Functions/CompareFunction.php +++ b/src/Collection/Functions/CompareFunction.php @@ -10,7 +10,6 @@ use Nextras\Dbal\QueryBuilder\QueryBuilder; use Nextras\Orm\Collection\Helpers\ArrayCollectionHelper; -use Nextras\Orm\Collection\Helpers\ConditionParserHelper; use Nextras\Orm\Collection\Helpers\DbalExpressionResult; use Nextras\Orm\Collection\Helpers\DbalQueryBuilderHelper; use Nextras\Orm\Entity\IEntity; @@ -19,6 +18,14 @@ class CompareFunction implements IArrayFunction, IQueryBuilderFunction { + public const OPERATOR_EQUAL = '='; + public const OPERATOR_NOT_EQUAL = '!='; + public const OPERATOR_GREATER = '>'; + public const OPERATOR_EQUAL_OR_GREATER = '>='; + public const OPERATOR_SMALLER = '<'; + public const OPERATOR_EQUAL_OR_SMALLER = '<='; + + public function processArrayExpression(ArrayCollectionHelper $helper, IEntity $entity, array $args) { assert(count($args) === 3); @@ -45,25 +52,25 @@ public function processArrayExpression(ArrayCollectionHelper $helper, IEntity $e private function arrayEvaluate(string $operator, $targetValue, $sourceValue): bool { - if ($operator === ConditionParserHelper::OPERATOR_EQUAL) { + if ($operator === self::OPERATOR_EQUAL) { if (is_array($targetValue)) { return in_array($sourceValue, $targetValue, true); } else { return $sourceValue === $targetValue; } - } elseif ($operator === ConditionParserHelper::OPERATOR_NOT_EQUAL) { + } elseif ($operator === self::OPERATOR_NOT_EQUAL) { if (is_array($targetValue)) { return !in_array($sourceValue, $targetValue, true); } else { return $sourceValue !== $targetValue; } - } elseif ($operator === ConditionParserHelper::OPERATOR_GREATER) { + } elseif ($operator === self::OPERATOR_GREATER) { return $sourceValue > $targetValue; - } elseif ($operator === ConditionParserHelper::OPERATOR_EQUAL_OR_GREATER) { + } elseif ($operator === self::OPERATOR_EQUAL_OR_GREATER) { return $sourceValue >= $targetValue; - } elseif ($operator === ConditionParserHelper::OPERATOR_SMALLER) { + } elseif ($operator === self::OPERATOR_SMALLER) { return $sourceValue < $targetValue; - } elseif ($operator === ConditionParserHelper::OPERATOR_EQUAL_OR_SMALLER) { + } elseif ($operator === self::OPERATOR_EQUAL_OR_SMALLER) { return $sourceValue <= $targetValue; } else { throw new InvalidArgumentException(); @@ -105,7 +112,7 @@ public function processQueryBuilderExpression( $columns = null; } - if ($operator === ConditionParserHelper::OPERATOR_EQUAL) { + if ($operator === self::OPERATOR_EQUAL) { if (\is_array($value)) { if ($value) { if ($columns !== null) { @@ -125,7 +132,7 @@ public function processQueryBuilderExpression( return $expression->append('= %any', $value); } - } elseif ($operator === ConditionParserHelper::OPERATOR_NOT_EQUAL) { + } elseif ($operator === self::OPERATOR_NOT_EQUAL) { if (\is_array($value)) { if ($value) { if ($columns !== null) { diff --git a/src/Collection/Helpers/ConditionParserHelper.php b/src/Collection/Helpers/ConditionParserHelper.php index 3681247b..ef883274 100644 --- a/src/Collection/Helpers/ConditionParserHelper.php +++ b/src/Collection/Helpers/ConditionParserHelper.php @@ -8,26 +8,19 @@ namespace Nextras\Orm\Collection\Helpers; +use Nextras\Orm\Collection\Functions\CompareFunction; use Nextras\Orm\Entity\IEntity; use Nextras\Orm\InvalidArgumentException; class ConditionParserHelper { - public const OPERATOR_EQUAL = '='; - public const OPERATOR_NOT_EQUAL = '!='; - public const OPERATOR_GREATER = '>'; - public const OPERATOR_EQUAL_OR_GREATER = '>='; - public const OPERATOR_SMALLER = '<'; - public const OPERATOR_EQUAL_OR_SMALLER = '<='; - - public static function parsePropertyOperator(string $condition): array { if (!\preg_match('#^(.+?)(!=|<=|>=|=|>|<)?$#', $condition, $matches)) { - return [$condition, self::OPERATOR_EQUAL]; + return [$condition, CompareFunction::OPERATOR_EQUAL]; } else { - return [$matches[1], $matches[2] ?? self::OPERATOR_EQUAL]; + return [$matches[1], $matches[2] ?? CompareFunction::OPERATOR_EQUAL]; } } diff --git a/tests/cases/unit/Mapper/Dbal/DbalValueOperatorFunctionTest.phpt b/tests/cases/unit/Mapper/Dbal/DbalValueOperatorFunctionTest.phpt index 326d12be..6c82330c 100644 --- a/tests/cases/unit/Mapper/Dbal/DbalValueOperatorFunctionTest.phpt +++ b/tests/cases/unit/Mapper/Dbal/DbalValueOperatorFunctionTest.phpt @@ -9,7 +9,6 @@ namespace NextrasTests\Orm\Mapper\Dbal; use Mockery; use Nextras\Dbal\QueryBuilder\QueryBuilder; use Nextras\Orm\Collection\Functions\CompareFunction; -use Nextras\Orm\Collection\Helpers\ConditionParserHelper; use Nextras\Orm\Collection\Helpers\DbalExpressionResult; use Nextras\Orm\Collection\Helpers\DbalQueryBuilderHelper; use NextrasTests\Orm\TestCase; @@ -46,11 +45,11 @@ class DbalValueOperatorFunctionTest extends TestCase protected function operatorTestProvider() { return [ - [['%ex = %any', ['%column', 'books.id'], 1], [ConditionParserHelper::OPERATOR_EQUAL, 'id', 1]], - [['%ex != %any', ['%column', 'books.id'], 1], [ConditionParserHelper::OPERATOR_NOT_EQUAL, 'id', 1]], - [['%ex IN %any', ['%column', 'books.id'], [1, 2]], [ConditionParserHelper::OPERATOR_EQUAL, 'id', [1, 2]]], - [['%ex NOT IN %any', ['%column', 'books.id'], [1, 2]], [ConditionParserHelper::OPERATOR_NOT_EQUAL, 'id', [1, 2]]], - [['%ex IS NOT NULL', ['%column', 'books.id']], [ConditionParserHelper::OPERATOR_NOT_EQUAL, 'id', null]], + [['%ex = %any', ['%column', 'books.id'], 1], [CompareFunction::OPERATOR_EQUAL, 'id', 1]], + [['%ex != %any', ['%column', 'books.id'], 1], [CompareFunction::OPERATOR_NOT_EQUAL, 'id', 1]], + [['%ex IN %any', ['%column', 'books.id'], [1, 2]], [CompareFunction::OPERATOR_EQUAL, 'id', [1, 2]]], + [['%ex NOT IN %any', ['%column', 'books.id'], [1, 2]], [CompareFunction::OPERATOR_NOT_EQUAL, 'id', [1, 2]]], + [['%ex IS NOT NULL', ['%column', 'books.id']], [CompareFunction::OPERATOR_NOT_EQUAL, 'id', null]], ]; } } From 05bcab074d3b4c3b0d278c1ab1a650fe9591fca9 Mon Sep 17 00:00:00 2001 From: Jan Skrasek Date: Sun, 16 Feb 2020 15:00:07 +0100 Subject: [PATCH 6/6] collection: swap property expression & operator in CompareFunction --- src/Collection/Functions/CompareFunction.php | 11 +- .../Functions/ConjunctionOperatorFunction.php | 2 +- .../Functions/DisjunctionOperatorFunction.php | 2 +- .../Collection/collection.aggregation.phpt | 12 +- .../collection.aggregation.phpt.bak | 137 ++++++++++++++++++ .../Dbal/DbalValueOperatorFunctionTest.phpt | 10 +- 6 files changed, 156 insertions(+), 18 deletions(-) create mode 100644 tests/cases/integration/Collection/collection.aggregation.phpt.bak diff --git a/src/Collection/Functions/CompareFunction.php b/src/Collection/Functions/CompareFunction.php index 487cc4b4..f26908f2 100644 --- a/src/Collection/Functions/CompareFunction.php +++ b/src/Collection/Functions/CompareFunction.php @@ -28,9 +28,10 @@ class CompareFunction implements IArrayFunction, IQueryBuilderFunction public function processArrayExpression(ArrayCollectionHelper $helper, IEntity $entity, array $args) { - assert(count($args) === 3); - $operator = $args[0]; - $valueReference = $helper->getValue($entity, $args[1]); + \assert(\count($args) === 3); + $operator = $args[1]; + + $valueReference = $helper->getValue($entity, $args[0]); if ($valueReference->propertyMetadata !== null) { $targetValue = $helper->normalizeValue($args[2], $valueReference->propertyMetadata, true); } else { @@ -89,8 +90,8 @@ public function processQueryBuilderExpression( { \assert(\count($args) === 3); - $operator = $args[0]; - $expression = $helper->processPropertyExpr($builder, $args[1]); + $operator = $args[1]; + $expression = $helper->processPropertyExpr($builder, $args[0]); if ($expression->valueNormalizer !== null) { $cb = $expression->valueNormalizer; diff --git a/src/Collection/Functions/ConjunctionOperatorFunction.php b/src/Collection/Functions/ConjunctionOperatorFunction.php index 355aca5d..23dedac9 100644 --- a/src/Collection/Functions/ConjunctionOperatorFunction.php +++ b/src/Collection/Functions/ConjunctionOperatorFunction.php @@ -61,7 +61,7 @@ private function normalizeFunctions(array $args): array $processedArgs = []; foreach ($args as $argName => $argValue) { [$argName, $operator] = ConditionParserHelper::parsePropertyOperator($argName); - $processedArgs[] = [CompareFunction::class, $operator, $argName, $argValue]; + $processedArgs[] = [CompareFunction::class, $argName, $operator, $argValue]; } return $processedArgs; } diff --git a/src/Collection/Functions/DisjunctionOperatorFunction.php b/src/Collection/Functions/DisjunctionOperatorFunction.php index 67ebccb9..743be5a3 100644 --- a/src/Collection/Functions/DisjunctionOperatorFunction.php +++ b/src/Collection/Functions/DisjunctionOperatorFunction.php @@ -61,7 +61,7 @@ private function normalizeFunctions(array $args): array $processedArgs = []; foreach ($args as $argName => $argValue) { [$argName, $operator] = ConditionParserHelper::parsePropertyOperator($argName); - $processedArgs[] = [CompareFunction::class, $operator, $argName, $argValue]; + $processedArgs[] = [CompareFunction::class, $argName, $operator, $argValue]; } return $processedArgs; } diff --git a/tests/cases/integration/Collection/collection.aggregation.phpt b/tests/cases/integration/Collection/collection.aggregation.phpt index 22f7b37b..400d786b 100644 --- a/tests/cases/integration/Collection/collection.aggregation.phpt +++ b/tests/cases/integration/Collection/collection.aggregation.phpt @@ -28,8 +28,8 @@ class CollectionAggregationTest extends DataTestCase $booksId = $this->orm->authors ->findBy([ CompareFunction::class, - '<', [AvgAggregateFunction::class, 'books->price->cents'], + CompareFunction::OPERATOR_SMALLER, 110 ]) ->orderBy('id') @@ -39,8 +39,8 @@ class CollectionAggregationTest extends DataTestCase $booksId = $this->orm->authors ->findBy([ CompareFunction::class, - '<=', [AvgAggregateFunction::class, 'books->price->cents'], + CompareFunction::OPERATOR_EQUAL_OR_SMALLER, 120 ]) ->orderBy('id') @@ -54,8 +54,8 @@ class CollectionAggregationTest extends DataTestCase ->findAll() ->findBy([ CompareFunction::class, - '>=', [CountAggregateFunction::class, 'tags->id'], + CompareFunction::OPERATOR_EQUAL_OR_GREATER, 2, ]) ->orderBy('id') @@ -83,8 +83,8 @@ class CollectionAggregationTest extends DataTestCase $userIds = $this->orm->authors ->findBy([ CompareFunction::class, - '>', [MaxAggregateFunction::class, 'books->price->cents'], + CompareFunction::OPERATOR_GREATER, 150 ]) ->orderBy('id') @@ -98,8 +98,8 @@ class CollectionAggregationTest extends DataTestCase $userIds = $this->orm->authors ->findBy([ CompareFunction::class, - '<', [MinAggregateFunction::class, 'books->price->cents'], + CompareFunction::OPERATOR_SMALLER, 50 ]) ->orderBy('id') @@ -113,8 +113,8 @@ class CollectionAggregationTest extends DataTestCase $userIds = $this->orm->authors ->findBy([ CompareFunction::class, - '<=', [SumAggregateFunction::class, 'books->price->cents'], + CompareFunction::OPERATOR_EQUAL_OR_SMALLER, 200 ]) ->orderBy('id') diff --git a/tests/cases/integration/Collection/collection.aggregation.phpt.bak b/tests/cases/integration/Collection/collection.aggregation.phpt.bak new file mode 100644 index 00000000..6a38b8c5 --- /dev/null +++ b/tests/cases/integration/Collection/collection.aggregation.phpt.bak @@ -0,0 +1,137 @@ +>>>>>> 01ab8ac... collection: rename ValueOperatorFunction to CompareFunction +use Nextras\Orm\Collection\ICollection; +use NextrasTests\Orm\DataTestCase; +use Tester\Assert; + + +$dic = require_once __DIR__ . '/../../../bootstrap.php'; + + +class CollectionAggregationTest extends DataTestCase +{ + public function testAvg() + { + $booksId = $this->orm->authors + ->findBy([ + CompareFunction::class, + '<', + [AvgAggregateFunction::class, 'books->price->cents'], + 110 + ]) + ->orderBy('id') + ->fetchPairs(null, 'id'); + Assert::same([1], $booksId); + + $booksId = $this->orm->authors + ->findBy([ + CompareFunction::class, + '<=', + [AvgAggregateFunction::class, 'books->price->cents'], + 120 + ]) + ->orderBy('id') + ->fetchPairs(null, 'id'); + Assert::same([1, 2], $booksId); + } + + public function testCount() + { + $bookIds = $this->orm->books + ->findAll() + ->findBy([ + CompareFunction::class, + '>=', + [CountAggregateFunction::class, 'tags->id'], + 2, + ]) + ->orderBy('id') + ->fetchPairs(null, 'id'); + Assert::same([1, 2], $bookIds); + + $bookIds = $this->orm->books + ->findAll() + ->orderBy([CountAggregateFunction::class, 'tags->id']) + ->orderBy('id') + ->fetchPairs(null, 'id'); + Assert::same([4, 3, 1, 2], $bookIds); + + $bookIds = $this->orm->books + ->findAll() + ->orderBy([CountAggregateFunction::class, 'tags->id'], ICollection::DESC) + ->orderBy('id', ICollection::DESC) + ->fetchPairs(null, 'id'); + Assert::same([2, 1, 3, 4], $bookIds); + } + + + public function testMax() + { + $userIds = $this->orm->authors + ->findBy([ + CompareFunction::class, + '>', + [MaxAggregateFunction::class, 'books->price->cents'], + 150 + ]) + ->orderBy('id') + ->fetchPairs(null, 'id'); + Assert::same([2], $userIds); + } + + + public function testMin() + { + $userIds = $this->orm->authors + ->findBy([ + CompareFunction::class, + '<', + [MinAggregateFunction::class, 'books->price->cents'], + 50 + ]) + ->orderBy('id') + ->fetchPairs(null, 'id'); + Assert::same([2], $userIds); + } + + + public function testSum() + { + $userIds = $this->orm->authors + ->findBy([ + CompareFunction::class, + '<=', + [SumAggregateFunction::class, 'books->price->cents'], + 200 + ]) + ->orderBy('id') + ->fetchPairs(null, 'id'); + Assert::same([1], $userIds); + } +} + + +$test = new CollectionAggregationTest($dic); +$test->run(); diff --git a/tests/cases/unit/Mapper/Dbal/DbalValueOperatorFunctionTest.phpt b/tests/cases/unit/Mapper/Dbal/DbalValueOperatorFunctionTest.phpt index 6c82330c..e776ff9c 100644 --- a/tests/cases/unit/Mapper/Dbal/DbalValueOperatorFunctionTest.phpt +++ b/tests/cases/unit/Mapper/Dbal/DbalValueOperatorFunctionTest.phpt @@ -45,11 +45,11 @@ class DbalValueOperatorFunctionTest extends TestCase protected function operatorTestProvider() { return [ - [['%ex = %any', ['%column', 'books.id'], 1], [CompareFunction::OPERATOR_EQUAL, 'id', 1]], - [['%ex != %any', ['%column', 'books.id'], 1], [CompareFunction::OPERATOR_NOT_EQUAL, 'id', 1]], - [['%ex IN %any', ['%column', 'books.id'], [1, 2]], [CompareFunction::OPERATOR_EQUAL, 'id', [1, 2]]], - [['%ex NOT IN %any', ['%column', 'books.id'], [1, 2]], [CompareFunction::OPERATOR_NOT_EQUAL, 'id', [1, 2]]], - [['%ex IS NOT NULL', ['%column', 'books.id']], [CompareFunction::OPERATOR_NOT_EQUAL, 'id', null]], + [['%ex = %any', ['%column', 'books.id'], 1], ['id', CompareFunction::OPERATOR_EQUAL, 1]], + [['%ex != %any', ['%column', 'books.id'], 1], ['id', CompareFunction::OPERATOR_NOT_EQUAL, 1]], + [['%ex IN %any', ['%column', 'books.id'], [1, 2]], ['id', CompareFunction::OPERATOR_EQUAL, [1, 2]]], + [['%ex NOT IN %any', ['%column', 'books.id'], [1, 2]], ['id', CompareFunction::OPERATOR_NOT_EQUAL, [1, 2]]], + [['%ex IS NOT NULL', ['%column', 'books.id']], ['id', CompareFunction::OPERATOR_NOT_EQUAL, null]], ]; } }