Skip to content

Commit 6721baa

Browse files
vincentchalamonvincentchalamon
committed
Add tests
- Fix from GitHub comments - Rename Request & Query factory class - Fix request get content
1 parent b4074ef commit 6721baa

29 files changed

+1910
-2125
lines changed

‎CHANGELOG.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
# Changelog
22

3-
## 1.0.0 beta 4
3+
## 1.1.0
44

55
* Support nested properties in Doctrine filters
66
* Add method to avoid naming collision of DQL join alias and bound parameter name

‎Doctrine/Orm/DataProvider.php

Lines changed: 46 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
use Doctrine\ORM\Tools\Pagination\Paginator as DoctrineOrmPaginator;
1717
use Doctrine\ORM\QueryBuilder;
1818
use Dunglas\ApiBundle\Doctrine\Orm\Filter\FilterInterface;
19+
use Dunglas\ApiBundle\Doctrine\Orm\Util\QueryChecker;
1920
use Dunglas\ApiBundle\Model\DataProviderInterface;
2021
use Dunglas\ApiBundle\Api\ResourceInterface;
2122
use Symfony\Component\HttpFoundation\Request;
@@ -140,6 +141,14 @@ public function getCollection(ResourceInterface $resource, Request $request)
140141
return $this->getPaginator($queryBuilder);
141142
}
142143

144+
/**
145+
* {@inheritdoc}
146+
*/
147+
public function supports(ResourceInterface $resource)
148+
{
149+
return null !== $this->managerRegistry->getManagerForClass($resource->getEntityClass());
150+
}
151+
143152
/**
144153
* Gets the paginator.
145154
*
@@ -151,16 +160,49 @@ protected function getPaginator(QueryBuilder $queryBuilder)
151160
{
152161
$doctrineOrmPaginator = new DoctrineOrmPaginator($queryBuilder);
153162
// Disable output walkers by default (performance)
154-
$doctrineOrmPaginator->setUseOutputWalkers(false);
163+
$doctrineOrmPaginator->setUseOutputWalkers($this->useOutputWalkers($queryBuilder));
155164

156165
return new Paginator($doctrineOrmPaginator);
157166
}
158167

159168
/**
160-
* {@inheritdoc}
169+
* Determines whether output walkers should be used.
170+
*
171+
* @param QueryBuilder $queryBuilder
172+
*
173+
* @return bool
161174
*/
162-
public function supports(ResourceInterface $resource)
175+
private function useOutputWalkers(QueryBuilder $queryBuilder)
163176
{
164-
return null !== $this->managerRegistry->getManagerForClass($resource->getEntityClass());
177+
/*
178+
* "Cannot count query that uses a HAVING clause. Use the output walkers for pagination"
179+
*
180+
* @see https://github.com/doctrine/doctrine2/blob/900b55d16afdcdeb5100d435a7166d3a425b9873/lib/Doctrine/ORM/Tools/Pagination/CountWalker.php#L50
181+
*/
182+
if (QueryChecker::hasHavingClause($queryBuilder)) {
183+
return true;
184+
}
185+
/*
186+
* "Paginating an entity with foreign key as identifier only works when using the Output Walkers. Call Paginator#setUseOutputWalkers(true) before iterating the paginator."
187+
*
188+
* @see https://github.com/doctrine/doctrine2/blob/900b55d16afdcdeb5100d435a7166d3a425b9873/lib/Doctrine/ORM/Tools/Pagination/LimitSubqueryWalker.php#L87
189+
*/
190+
if (QueryChecker::hasRootEntityWithForeignKeyIdentifier($queryBuilder, $this->managerRegistry)) {
191+
return true;
192+
}
193+
/*
194+
* "Cannot select distinct identifiers from query with LIMIT and ORDER BY on a column from a fetch joined to-many association. Use output walkers."
195+
*
196+
* @see https://github.com/doctrine/doctrine2/blob/900b55d16afdcdeb5100d435a7166d3a425b9873/lib/Doctrine/ORM/Tools/Pagination/LimitSubqueryWalker.php#L149
197+
*/
198+
if (
199+
QueryChecker::hasMaxResults($queryBuilder)
200+
&& QueryChecker::hasOrderByOnToManyJoin($queryBuilder, $this->managerRegistry)
201+
) {
202+
return true;
203+
}
204+
205+
// Disable output walkers by default (performance)
206+
return false;
165207
}
166208
}

‎Doctrine/Orm/Filter/AbstractFilter.php

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414
use Doctrine\Common\Persistence\ManagerRegistry;
1515
use Doctrine\Common\Persistence\Mapping\ClassMetadata;
1616
use Dunglas\ApiBundle\Api\ResourceInterface;
17-
use Dunglas\ApiBundle\Util\RequestUtils;
17+
use Dunglas\ApiBundle\Util\RequestParser;
1818
use Symfony\Component\HttpFoundation\Request;
1919

2020
/**
@@ -24,6 +24,7 @@
2424
*
2525
* @author Kévin Dunglas <[email protected]>
2626
* @author Théo FIDRY <[email protected]>
27+
* @author Vincent Chalamon <[email protected]>
2728
*/
2829
abstract class AbstractFilter implements FilterInterface
2930
{
@@ -180,7 +181,7 @@ protected function extractProperties(Request $request)
180181
}
181182

182183
if ($needsFixing) {
183-
$request = RequestUtils::getFixedRequest($request);
184+
$request = RequestParser::parseAndDuplicateRequest($request);
184185
}
185186

186187
return $request->query->all();

‎Doctrine/Orm/Filter/DateFilter.php

Lines changed: 32 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313

1414
use Doctrine\ORM\QueryBuilder;
1515
use Dunglas\ApiBundle\Api\ResourceInterface;
16-
use Dunglas\ApiBundle\Doctrine\Orm\Util\QueryUtils;
16+
use Dunglas\ApiBundle\Doctrine\Orm\Util\QueryNameGenerator;
1717
use Symfony\Component\HttpFoundation\Request;
1818

1919
/**
@@ -46,12 +46,10 @@ class DateFilter extends AbstractFilter
4646
*/
4747
public function apply(ResourceInterface $resource, QueryBuilder $queryBuilder, Request $request)
4848
{
49-
$fieldNames = $this->getDateFieldNames($resource);
50-
5149
foreach ($this->extractProperties($request) as $property => $values) {
5250
// Expect $values to be an array having the period as keys and the date value as values
5351
if (
54-
!isset($fieldNames[$property]) ||
52+
!$this->isDateField($property, $resource) ||
5553
!is_array($values) ||
5654
!$this->isPropertyEnabled($property) ||
5755
!$this->isPropertyMapped($property, $resource)
@@ -68,7 +66,7 @@ public function apply(ResourceInterface $resource, QueryBuilder $queryBuilder, R
6866
$parentAlias = $alias;
6967

7068
foreach ($propertyParts['associations'] as $association) {
71-
$alias = QueryUtils::generateJoinAlias($association);
69+
$alias = QueryNameGenerator::generateJoinAlias($association);
7270
$queryBuilder->join(sprintf('%s.%s', $parentAlias, $association), $alias);
7371
$parentAlias = $alias;
7472
}
@@ -78,10 +76,6 @@ public function apply(ResourceInterface $resource, QueryBuilder $queryBuilder, R
7876

7977
$nullManagement = isset($this->properties[$property]) ? $this->properties[$property] : null;
8078

81-
if (self::EXCLUDE_NULL === $nullManagement) {
82-
$queryBuilder->andWhere($queryBuilder->expr()->isNotNull(sprintf('%s.%s', $alias, $field)));
83-
}
84-
8579
if (isset($values[self::PARAMETER_BEFORE])) {
8680
$this->addWhere(
8781
$queryBuilder,
@@ -110,22 +104,21 @@ public function apply(ResourceInterface $resource, QueryBuilder $queryBuilder, R
110104
* Adds the where clause according to the chosen null management.
111105
*
112106
* @param QueryBuilder $queryBuilder
113-
* @param string $alias
114-
* @param string $field
115-
* @param string $operator
116-
* @param string $value
117-
* @param int|null $nullManagement
107+
* @param string $alias
108+
* @param string $field
109+
* @param string $operator
110+
* @param string $value
111+
* @param int|null $nullManagement
118112
*/
119113
private function addWhere(QueryBuilder $queryBuilder, $alias, $field, $operator, $value, $nullManagement)
120114
{
121-
$valueParameter = QueryUtils::generateParameterName(sprintf('%s_%s', $field, $operator));
115+
$valueParameter = QueryNameGenerator::generateParameterName(sprintf('%s_%s', $field, $operator));
122116
$baseWhere = sprintf('%s.%s %s :%s', $alias, $field, self::PARAMETER_BEFORE === $operator ? '<=' : '>=', $valueParameter);
123117

124118
if (null === $nullManagement || self::EXCLUDE_NULL === $nullManagement) {
125119
$queryBuilder->andWhere($baseWhere);
126120
} elseif (
127-
(self::PARAMETER_BEFORE === $operator && self::INCLUDE_NULL_BEFORE === $nullManagement)
128-
||
121+
(self::PARAMETER_BEFORE === $operator && self::INCLUDE_NULL_BEFORE === $nullManagement) ||
129122
(self::PARAMETER_AFTER === $operator && self::INCLUDE_NULL_AFTER === $nullManagement)
130123
) {
131124
$queryBuilder->andWhere($queryBuilder->expr()->orX(
@@ -140,6 +133,10 @@ private function addWhere(QueryBuilder $queryBuilder, $alias, $field, $operator,
140133
}
141134

142135
$queryBuilder->setParameter($valueParameter, new \DateTime($value));
136+
137+
if (self::EXCLUDE_NULL === $nullManagement) {
138+
$queryBuilder->andWhere($queryBuilder->expr()->isNotNull(sprintf('%s.%s', $alias, $field)));
139+
}
143140
}
144141

145142
/**
@@ -148,11 +145,19 @@ private function addWhere(QueryBuilder $queryBuilder, $alias, $field, $operator,
148145
public function getDescription(ResourceInterface $resource)
149146
{
150147
$description = [];
151-
foreach ($this->getClassMetadata($resource)->getFieldNames() as $fieldName) {
152-
if ($this->isPropertyEnabled($fieldName)) {
153-
$description += $this->getFilterDescription($fieldName, self::PARAMETER_BEFORE);
154-
$description += $this->getFilterDescription($fieldName, self::PARAMETER_AFTER);
148+
149+
$properties = $this->properties;
150+
if (null === $properties) {
151+
$properties = array_fill_keys($this->getClassMetadata($resource)->getFieldNames(), null);
152+
}
153+
154+
foreach ($properties as $property => $nullManagement) {
155+
if (!$this->isPropertyMapped($property, $resource) || !$this->isDateField($property, $resource)) {
156+
continue;
155157
}
158+
159+
$description += $this->getFilterDescription($property, self::PARAMETER_BEFORE);
160+
$description += $this->getFilterDescription($property, self::PARAMETER_AFTER);
156161
}
157162

158163
return $description;
@@ -178,23 +183,18 @@ private function getFilterDescription($fieldName, $period)
178183
}
179184

180185
/**
181-
* Gets names of fields with a date type.
186+
* Determines whether the given property refers to a date field.
182187
*
188+
* @param string $property
183189
* @param ResourceInterface $resource
184190
*
185191
* @return array
186192
*/
187-
private function getDateFieldNames(ResourceInterface $resource)
193+
private function isDateField($property, ResourceInterface $resource)
188194
{
189-
$classMetadata = $this->getClassMetadata($resource);
190-
$dateFieldNames = [];
191-
192-
foreach ($classMetadata->getFieldNames() as $fieldName) {
193-
if (isset(self::$doctrineDateTypes[$classMetadata->getTypeOfField($fieldName)])) {
194-
$dateFieldNames[$fieldName] = true;
195-
}
196-
}
195+
$propertyParts = $this->splitPropertyParts($property);
196+
$metadata = $this->getNestedMetadata($resource, $propertyParts['associations']);
197197

198-
return $dateFieldNames;
198+
return isset(self::$doctrineDateTypes[$metadata->getTypeOfField($propertyParts['field'])]);
199199
}
200200
}

‎Doctrine/Orm/Filter/OrderFilter.php

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,14 +14,15 @@
1414
use Doctrine\Common\Persistence\ManagerRegistry;
1515
use Doctrine\ORM\QueryBuilder;
1616
use Dunglas\ApiBundle\Api\ResourceInterface;
17-
use Dunglas\ApiBundle\Doctrine\Orm\Util\QueryUtils;
17+
use Dunglas\ApiBundle\Doctrine\Orm\Util\QueryNameGenerator;
1818
use Symfony\Component\HttpFoundation\Request;
1919

2020
/**
2121
* Order the collection by given properties.
2222
*
2323
* @author Théo FIDRY <[email protected]>
2424
* @author Kévin Dunglas <[email protected]>
25+
* @author Vincent Chalamon <[email protected]>
2526
*/
2627
class OrderFilter extends AbstractFilter
2728
{
@@ -59,7 +60,7 @@ public function apply(ResourceInterface $resource, QueryBuilder $queryBuilder, R
5960
continue;
6061
}
6162

62-
if ('' === $order && isset($this->properties[$property])) {
63+
if (empty($order) && isset($this->properties[$property])) {
6364
$order = $this->properties[$property];
6465
}
6566

@@ -77,7 +78,7 @@ public function apply(ResourceInterface $resource, QueryBuilder $queryBuilder, R
7778
$parentAlias = $alias;
7879

7980
foreach ($propertyParts['associations'] as $association) {
80-
$alias = QueryUtils::generateJoinAlias($association);
81+
$alias = QueryNameGenerator::generateJoinAlias($association);
8182
$queryBuilder->join(sprintf('%s.%s', $parentAlias, $association), $alias);
8283
$parentAlias = $alias;
8384
}

0 commit comments

Comments
 (0)