From 30f4de37ee263346077bc26a7b90c6fa0a85b3a8 Mon Sep 17 00:00:00 2001 From: Mariano Filipoff Date: Thu, 4 Jun 2020 10:23:57 -0300 Subject: [PATCH 1/8] QueryBuilderDecorator improvements. --- src/Doctrine/QueryBuilderDecorator.php | 224 ++++++++++++++++++++++++- 1 file changed, 215 insertions(+), 9 deletions(-) diff --git a/src/Doctrine/QueryBuilderDecorator.php b/src/Doctrine/QueryBuilderDecorator.php index 8d13410..5f61af6 100644 --- a/src/Doctrine/QueryBuilderDecorator.php +++ b/src/Doctrine/QueryBuilderDecorator.php @@ -3,35 +3,106 @@ namespace Digbang\Utils\Doctrine; use Digbang\Utils\Sorting; +use Doctrine\ORM\Query\Expr; +use Doctrine\ORM\Query\Expr\From; +use Doctrine\ORM\Query\Expr\Join; use Doctrine\ORM\QueryBuilder; +use Illuminate\Support\Collection; +use Illuminate\Support\Str; class QueryBuilderDecorator extends QueryBuilder { public function __construct(QueryBuilder $queryBuilder) { parent::__construct($queryBuilder->getEntityManager()); + + $this->decorateDQLParts($queryBuilder->getDQLParts()); } /** - * @return QueryBuilderDecorator + * {@inheritDoc} + * + * @throws \InvalidArgumentException + */ + public function from($from, $alias, $indexBy = null) + { + $normalizedAlias = $this->normalizeAlias($alias); + $this->assertHasOneAlias($normalizedAlias); + $firstAlias = $normalizedAlias->first(); + + /** @var From $_from */ + foreach ($this->getDQLPart('from') as $_from) { + if (($_from->getAlias() === $firstAlias) && ($_from->getFrom() !== $from)) { + throw new \InvalidArgumentException("Duplicated FROM alias: $firstAlias."); + } + } + + return parent::from($from, $firstAlias, $indexBy); + } + + /** + * {@inheritDoc} */ public function select($select = null) { - /** @var static $queryBuilder */ - $queryBuilder = parent::select($select); + $selects = is_array($select) ? $select : func_get_args(); + $normalizedSelects = $this->normalizeAlias($selects); - return $queryBuilder; + return parent::select($normalizedSelects->toArray()); } /** - * @return QueryBuilderDecorator + * {@inheritDoc} */ - public function from($from, $alias, $indexBy = null) + public function addSelect($select = null) + { + $selects = is_array($select) ? $select : func_get_args(); + $normalizedSelects = $this->normalizeAlias($selects); + + /** @var Collection $dqlSelectParts */ + $dqlSelectParts = collect($this->getDQLPart('select')) + ->flatMap + ->getParts(); + + $newSelects = $normalizedSelects->diff($dqlSelectParts); + + return parent::addSelect($newSelects->toArray()); + } + + /** + * {@inheritDoc} + * + * @throws \InvalidArgumentException + */ + public function innerJoin($join, $alias, $conditionType = null, $condition = null, $indexBy = null) { - /** @var static $queryBuilder */ - $queryBuilder = parent::from($from, $alias, $indexBy); + $normalizedAlias = $this->normalizeAlias($alias); + $this->assertHasOneAlias($normalizedAlias); + $firstAlias = $normalizedAlias->first(); - return $queryBuilder; + if ($this->isJoined($alias, Join::INNER_JOIN)) { + return $this->mergeJoinConditions($alias, $conditionType, $condition); + } + + return parent::innerJoin($join, $firstAlias, $conditionType, $condition, $indexBy); + } + + /** + * {@inheritDoc} + * + * @throws \InvalidArgumentException + */ + public function leftJoin($join, $alias, $conditionType = null, $condition = null, $indexBy = null) + { + $normalizedAlias = $this->normalizeAlias($alias); + $this->assertHasOneAlias($normalizedAlias); + $firstAlias = $normalizedAlias->first(); + + if ($this->isJoined($alias, Join::LEFT_JOIN)) { + return $this->mergeJoinConditions($alias, $conditionType, $condition); + } + + return parent::leftJoin($join, $firstAlias, $conditionType, $condition, $indexBy); } /** @@ -123,4 +194,139 @@ public function applyFilters(array $filters): QueryBuilderDecorator return $this; } + + /** + * Pre: Receive a DQLParts array. + * + * Post: Fill the current instance DQLParts with the given ones. + * + * @param array $dqlParts + * + * @return void + */ + private function decorateDQLParts(array $dqlParts): void + { + $filledDQLParts = array_filter($dqlParts); + + foreach ($filledDQLParts as $key => $value) { + $this->add($key, $value, false); + } + } + + /** + * Pre: Receive an array or string like one of the following (with all it's variants): + * a) 'a, b, c' + * b) ['a as foo, b, c'] + * c) ['a as foo', 'b', 'c'] + * + * Post: Return a normalized array like the following: ['a', 'b', 'c'] + * + * Normalized array means no NULL, empty strings, duplicates or ' ' like should be inside of it. + * + * @param string|array $alias + * + * @return Collection + */ + private function normalizeAlias($alias): Collection + { + return Str::of(collect($alias)->join(',')) + ->explode(',') + ->map(function (string $value) { + return trim($value); + }) + ->filter() + ->unique() + ->values(); + } + + /** + * Pre: Receive an alias collection + * + * Post: Throw an \InvalidArgumentException if the alias is empty or has more than one alias + * + * @param Collection $alias + * + * @throws \InvalidArgumentException + */ + private function assertHasOneAlias(Collection $alias): void + { + if ($alias->count() != 1) { + throw new \InvalidArgumentException("There should be one alias on the current context."); + } + } + + /** + * Pre: Receive two strings: join alias and type. + * + * Post: Return a true if the join was defined before, otherwhise return false. \InvalidArgumentException may be + * thrown if the alias was defined for a different join type. + * + * @param string $alias + * @param string $joinType + * + * @throws \InvalidArgumentException + * + * @return bool + */ + protected function isJoined(string $alias, string $joinType): bool + { + /** @var Join[] $joins */ + $joins = collect($this->getDQLPart('join'))->flatten(); + + foreach ($joins as $join) { + if ($join->getAlias() !== $alias) { + continue; + } + + if ($join->getJoinType() === $joinType) { + return true; + } + + throw new \InvalidArgumentException("Alias '$alias' is defined for a different join type: {$join->getJoinType()}."); + } + + return false; + } + + /** + * Pre: Receive the alias, condition type and the join condition as strings. + * + * Post: Merge the given condition with the desired join. + * + * @param string $alias + * @param null|string $conditionType + * @param null|string|Expr\Comparison|Expr\Func|Expr\Orx $condition + * + * @return QueryBuilderDecorator + */ + protected function mergeJoinConditions(string $alias, ?string $conditionType = Join::WITH, $condition = null): self + { + if (is_null($condition)) { + return $this; + } + + /** @var Join[] $joins */ + foreach ($this->getDQLPart('join') as $key => $joins) { + foreach ($joins as $index => $join) { + if ($join->getAlias() !== $alias) { + continue; + } + + if (is_null($join->getConditionType()) || $join->getConditionType() === $conditionType) { + $joins[$index] = new Join( + $join->getJoinType(), + $join->getJoin(), + $join->getAlias(), + $conditionType, + (string)$this->expr()->andX($join->getCondition(), $condition), + $join->getIndexBy() + ); + + return $this->add('join', [$key => $joins], false); + } + } + } + + return $this; + } } From 1e94a43da86de0562ce513e5d967083ba5734775 Mon Sep 17 00:00:00 2001 From: Mariano Filipoff Date: Thu, 4 Jun 2020 10:51:25 -0300 Subject: [PATCH 2/8] QueryBuilderDecorator: small fix. --- src/Doctrine/QueryBuilderDecorator.php | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/Doctrine/QueryBuilderDecorator.php b/src/Doctrine/QueryBuilderDecorator.php index 5f61af6..def6552 100644 --- a/src/Doctrine/QueryBuilderDecorator.php +++ b/src/Doctrine/QueryBuilderDecorator.php @@ -6,6 +6,7 @@ use Doctrine\ORM\Query\Expr; use Doctrine\ORM\Query\Expr\From; use Doctrine\ORM\Query\Expr\Join; +use Doctrine\ORM\Query\Expr\Select; use Doctrine\ORM\QueryBuilder; use Illuminate\Support\Collection; use Illuminate\Support\Str; @@ -58,11 +59,12 @@ public function addSelect($select = null) { $selects = is_array($select) ? $select : func_get_args(); $normalizedSelects = $this->normalizeAlias($selects); + $dqlSelectParts = collect(); - /** @var Collection $dqlSelectParts */ - $dqlSelectParts = collect($this->getDQLPart('select')) - ->flatMap - ->getParts(); + /** @var Select $_select */ + foreach ($this->getDQLPart('select') as $_select) { + $dqlSelectParts = $dqlSelectParts->merge($this->normalizeAlias($_select->getParts())); + } $newSelects = $normalizedSelects->diff($dqlSelectParts); From df475b41ba08a81f5655deff9571d865a37eff83 Mon Sep 17 00:00:00 2001 From: Mariano Filipoff Date: Thu, 4 Jun 2020 11:13:23 -0300 Subject: [PATCH 3/8] QueryBuilderDecorator: new function: when --- src/Doctrine/QueryBuilderDecorator.php | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/src/Doctrine/QueryBuilderDecorator.php b/src/Doctrine/QueryBuilderDecorator.php index def6552..681bd67 100644 --- a/src/Doctrine/QueryBuilderDecorator.php +++ b/src/Doctrine/QueryBuilderDecorator.php @@ -107,6 +107,23 @@ public function leftJoin($join, $alias, $conditionType = null, $condition = null return parent::leftJoin($join, $firstAlias, $conditionType, $condition, $indexBy); } + /** + * Pre: Receive a value and a callback. + * + * Post: Execute the callback if the given value is not null. + * + * @param mixed $value + * @param \Closure $callback + */ + public function when($value, \Closure $callback): void + { + if (is_null($value)) { + return; + } + + $callback->__invoke($this, $value); + } + /** * Adds "order by" statement if sort is found in sortOptions. * Returns true if order by was added. From 0e85db3f75329a0d61f5de16ba910d65df15f75d Mon Sep 17 00:00:00 2001 From: Mariano Filipoff Date: Thu, 4 Jun 2020 11:14:29 -0300 Subject: [PATCH 4/8] QueryBuilderDecorator: now when returns the current QueryBuilderDecorator instance. --- src/Doctrine/QueryBuilderDecorator.php | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/Doctrine/QueryBuilderDecorator.php b/src/Doctrine/QueryBuilderDecorator.php index 681bd67..224daf5 100644 --- a/src/Doctrine/QueryBuilderDecorator.php +++ b/src/Doctrine/QueryBuilderDecorator.php @@ -114,14 +114,18 @@ public function leftJoin($join, $alias, $conditionType = null, $condition = null * * @param mixed $value * @param \Closure $callback + * + * @return QueryBuilderDecorator */ - public function when($value, \Closure $callback): void + public function when($value, \Closure $callback): self { if (is_null($value)) { - return; + return $this; } $callback->__invoke($this, $value); + + return $this; } /** From 3e89b211d36997b0f41578416ab24c64c0088b31 Mon Sep 17 00:00:00 2001 From: Mariano Filipoff Date: Thu, 4 Jun 2020 11:35:10 -0300 Subject: [PATCH 5/8] QueryBuilderDecorator: minor fix for JOIN merges --- src/Doctrine/QueryBuilderDecorator.php | 29 +++++++++++++------------- 1 file changed, 15 insertions(+), 14 deletions(-) diff --git a/src/Doctrine/QueryBuilderDecorator.php b/src/Doctrine/QueryBuilderDecorator.php index 224daf5..d61cf4a 100644 --- a/src/Doctrine/QueryBuilderDecorator.php +++ b/src/Doctrine/QueryBuilderDecorator.php @@ -291,7 +291,7 @@ private function assertHasOneAlias(Collection $alias): void * * @return bool */ - protected function isJoined(string $alias, string $joinType): bool + private function isJoined(string $alias, string $joinType): bool { /** @var Join[] $joins */ $joins = collect($this->getDQLPart('join'))->flatten(); @@ -322,7 +322,7 @@ protected function isJoined(string $alias, string $joinType): bool * * @return QueryBuilderDecorator */ - protected function mergeJoinConditions(string $alias, ?string $conditionType = Join::WITH, $condition = null): self + private function mergeJoinConditions(string $alias, ?string $conditionType = Join::WITH, $condition = null): self { if (is_null($condition)) { return $this; @@ -335,18 +335,19 @@ protected function mergeJoinConditions(string $alias, ?string $conditionType = J continue; } - if (is_null($join->getConditionType()) || $join->getConditionType() === $conditionType) { - $joins[$index] = new Join( - $join->getJoinType(), - $join->getJoin(), - $join->getAlias(), - $conditionType, - (string)$this->expr()->andX($join->getCondition(), $condition), - $join->getIndexBy() - ); - - return $this->add('join', [$key => $joins], false); - } + $newConditionType = in_array(Join::ON, [$join->getConditionType(), $conditionType]) ? Join::ON : Join::WITH; + $newCondition = $this->expr()->andX($join->getCondition(), $condition); + + $joins[$index] = new Join( + $join->getJoinType(), + $join->getJoin(), + $join->getAlias(), + $newConditionType, + $newCondition, + $join->getIndexBy() + ); + + return $this->add('join', [$key => $joins], false); } } From 22164e243a4642f92a72cf7e8b6d5dc270840943 Mon Sep 17 00:00:00 2001 From: Mariano Filipoff Date: Thu, 4 Jun 2020 13:42:53 -0300 Subject: [PATCH 6/8] QueryBuilderDecorator: review changes. --- src/Doctrine/QueryBuilderDecorator.php | 94 +++++++++++--------------- 1 file changed, 41 insertions(+), 53 deletions(-) diff --git a/src/Doctrine/QueryBuilderDecorator.php b/src/Doctrine/QueryBuilderDecorator.php index d61cf4a..785740c 100644 --- a/src/Doctrine/QueryBuilderDecorator.php +++ b/src/Doctrine/QueryBuilderDecorator.php @@ -27,18 +27,16 @@ public function __construct(QueryBuilder $queryBuilder) */ public function from($from, $alias, $indexBy = null) { - $normalizedAlias = $this->normalizeAlias($alias); - $this->assertHasOneAlias($normalizedAlias); - $firstAlias = $normalizedAlias->first(); + $singleAlias = $this->getSingleAlias($alias); - /** @var From $_from */ - foreach ($this->getDQLPart('from') as $_from) { - if (($_from->getAlias() === $firstAlias) && ($_from->getFrom() !== $from)) { - throw new \InvalidArgumentException("Duplicated FROM alias: $firstAlias."); + /** @var From $part */ + foreach ($this->getDQLPart('from') as $part) { + if (($part->getAlias() === $singleAlias) && ($part->getFrom() !== $from)) { + throw new \InvalidArgumentException("Duplicated FROM alias: $singleAlias."); } } - return parent::from($from, $firstAlias, $indexBy); + return parent::from($from, $singleAlias, $indexBy); } /** @@ -61,14 +59,15 @@ public function addSelect($select = null) $normalizedSelects = $this->normalizeAlias($selects); $dqlSelectParts = collect(); - /** @var Select $_select */ - foreach ($this->getDQLPart('select') as $_select) { - $dqlSelectParts = $dqlSelectParts->merge($this->normalizeAlias($_select->getParts())); + /** @var Select $part */ + foreach ($this->getDQLPart('select') as $part) { + $dqlSelectParts = $dqlSelectParts->merge($this->normalizeAlias($part->getParts())); } - $newSelects = $normalizedSelects->diff($dqlSelectParts); - - return parent::addSelect($newSelects->toArray()); + return parent::addSelect($normalizedSelects + ->diff($dqlSelectParts) + ->toArray() + ); } /** @@ -78,15 +77,13 @@ public function addSelect($select = null) */ public function innerJoin($join, $alias, $conditionType = null, $condition = null, $indexBy = null) { - $normalizedAlias = $this->normalizeAlias($alias); - $this->assertHasOneAlias($normalizedAlias); - $firstAlias = $normalizedAlias->first(); + $singleAlias = $this->getSingleAlias($alias); - if ($this->isJoined($alias, Join::INNER_JOIN)) { - return $this->mergeJoinConditions($alias, $conditionType, $condition); + if ($this->isJoined($singleAlias, Join::INNER_JOIN)) { + return $this->mergeJoinConditions($singleAlias, $conditionType, $condition); } - return parent::innerJoin($join, $firstAlias, $conditionType, $condition, $indexBy); + return parent::innerJoin($join, $singleAlias, $conditionType, $condition, $indexBy); } /** @@ -96,36 +93,13 @@ public function innerJoin($join, $alias, $conditionType = null, $condition = nul */ public function leftJoin($join, $alias, $conditionType = null, $condition = null, $indexBy = null) { - $normalizedAlias = $this->normalizeAlias($alias); - $this->assertHasOneAlias($normalizedAlias); - $firstAlias = $normalizedAlias->first(); + $singleAlias = $this->getSingleAlias($alias); - if ($this->isJoined($alias, Join::LEFT_JOIN)) { - return $this->mergeJoinConditions($alias, $conditionType, $condition); - } - - return parent::leftJoin($join, $firstAlias, $conditionType, $condition, $indexBy); - } - - /** - * Pre: Receive a value and a callback. - * - * Post: Execute the callback if the given value is not null. - * - * @param mixed $value - * @param \Closure $callback - * - * @return QueryBuilderDecorator - */ - public function when($value, \Closure $callback): self - { - if (is_null($value)) { - return $this; + if ($this->isJoined($singleAlias, Join::LEFT_JOIN)) { + return $this->mergeJoinConditions($singleAlias, $conditionType, $condition); } - $callback->__invoke($this, $value); - - return $this; + return parent::leftJoin($join, $singleAlias, $conditionType, $condition, $indexBy); } /** @@ -141,6 +115,8 @@ public function when($value, \Closure $callback): self * ] * ]. * + * @deprecated + * * @param Sorting $sorting * @param array $sortOptions * @@ -159,6 +135,8 @@ public function addSorting(Sorting $sorting, array $sortOptions): QueryBuilderDe * Adds "order by" statement with raw PaginationData sorting values * Returns true if order by was added. * + * @deprecated + * * @param Sorting $sorting * * @return QueryBuilderDecorator @@ -181,6 +159,8 @@ public function addRawSorting(Sorting $sorting): QueryBuilderDecorator * 'aliasJoinC' => 'aliasJoinA.fieldA', * ]. * + * @deprecated + * * @param array $joins * @param array|null $leftJoins * @@ -204,6 +184,8 @@ public function applyJoins(array $joins, array $leftJoins = null): QueryBuilderD /** * Adds "andWhere's" statements for each filter. * + * @deprecated + * * @param array $filters * * @return QueryBuilderDecorator @@ -263,19 +245,25 @@ private function normalizeAlias($alias): Collection } /** - * Pre: Receive an alias collection + * Pre: Receive an alias * - * Post: Throw an \InvalidArgumentException if the alias is empty or has more than one alias + * Post: Return the first normalized alias if it has only one, otherwise thrown an \InvalidArgumentException if the alias is empty or has more than one alias * - * @param Collection $alias + * @param null|string|array $alias * * @throws \InvalidArgumentException + * + * @return string */ - private function assertHasOneAlias(Collection $alias): void + private function getSingleAlias($alias): string { - if ($alias->count() != 1) { - throw new \InvalidArgumentException("There should be one alias on the current context."); + $normalizedAlias = $this->normalizeAlias($alias); + + if ($normalizedAlias->count() != 1) { + throw new \InvalidArgumentException('There should be one alias on the current context.'); } + + return $normalizedAlias->first(); } /** From d89c09d64d9c1dc6c22d4ca9a10f5eb9168d7d92 Mon Sep 17 00:00:00 2001 From: Mariano Filipoff Date: Thu, 4 Jun 2020 14:08:23 -0300 Subject: [PATCH 7/8] QueryBuilderDecorator: review changes v2. --- src/Doctrine/QueryBuilderDecorator.php | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/src/Doctrine/QueryBuilderDecorator.php b/src/Doctrine/QueryBuilderDecorator.php index 785740c..51c6147 100644 --- a/src/Doctrine/QueryBuilderDecorator.php +++ b/src/Doctrine/QueryBuilderDecorator.php @@ -31,7 +31,7 @@ public function from($from, $alias, $indexBy = null) /** @var From $part */ foreach ($this->getDQLPart('from') as $part) { - if (($part->getAlias() === $singleAlias) && ($part->getFrom() !== $from)) { + if ($part->getAlias() === $singleAlias && $part->getFrom() !== $from) { throw new \InvalidArgumentException("Duplicated FROM alias: $singleAlias."); } } @@ -115,8 +115,6 @@ public function leftJoin($join, $alias, $conditionType = null, $condition = null * ] * ]. * - * @deprecated - * * @param Sorting $sorting * @param array $sortOptions * @@ -135,8 +133,6 @@ public function addSorting(Sorting $sorting, array $sortOptions): QueryBuilderDe * Adds "order by" statement with raw PaginationData sorting values * Returns true if order by was added. * - * @deprecated - * * @param Sorting $sorting * * @return QueryBuilderDecorator @@ -184,8 +180,6 @@ public function applyJoins(array $joins, array $leftJoins = null): QueryBuilderD /** * Adds "andWhere's" statements for each filter. * - * @deprecated - * * @param array $filters * * @return QueryBuilderDecorator @@ -259,7 +253,7 @@ private function getSingleAlias($alias): string { $normalizedAlias = $this->normalizeAlias($alias); - if ($normalizedAlias->count() != 1) { + if ($normalizedAlias->count() !== 1) { throw new \InvalidArgumentException('There should be one alias on the current context.'); } From 606529da6b10b61154f3141eb813003e49a2cb8a Mon Sep 17 00:00:00 2001 From: Mariano Filipoff Date: Mon, 8 Jun 2020 10:13:19 -0300 Subject: [PATCH 8/8] QueryBuilderDecorator: small fix for select and addSelect methods. --- src/Doctrine/QueryBuilderDecorator.php | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/src/Doctrine/QueryBuilderDecorator.php b/src/Doctrine/QueryBuilderDecorator.php index 51c6147..490e6e1 100644 --- a/src/Doctrine/QueryBuilderDecorator.php +++ b/src/Doctrine/QueryBuilderDecorator.php @@ -45,9 +45,11 @@ public function from($from, $alias, $indexBy = null) public function select($select = null) { $selects = is_array($select) ? $select : func_get_args(); - $normalizedSelects = $this->normalizeAlias($selects); - return parent::select($normalizedSelects->toArray()); + return parent::select(collect($selects) + ->unique() + ->toArray() + ); } /** @@ -56,15 +58,17 @@ public function select($select = null) public function addSelect($select = null) { $selects = is_array($select) ? $select : func_get_args(); - $normalizedSelects = $this->normalizeAlias($selects); $dqlSelectParts = collect(); /** @var Select $part */ foreach ($this->getDQLPart('select') as $part) { - $dqlSelectParts = $dqlSelectParts->merge($this->normalizeAlias($part->getParts())); + foreach ($part->getParts() as $element) { + $dqlSelectParts->add($element); + } } - return parent::addSelect($normalizedSelects + return parent::addSelect(collect($selects) + ->unique() ->diff($dqlSelectParts) ->toArray() );