Skip to content

Commit

Permalink
Issue #2 Does this work with Collections?
Browse files Browse the repository at this point in the history
- added two workarounds
- adapted tests
- adapted readme
  • Loading branch information
metaclass-nl committed Dec 18, 2021
1 parent fa62291 commit d48969d
Show file tree
Hide file tree
Showing 8 changed files with 365 additions and 35 deletions.
74 changes: 61 additions & 13 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,12 @@ Because of the nesting of or the criteria for the description are combined toget
AND with the criterium for price, which must allways be true while only one of the
criteria for the desciption needs to be true for an order to be returned.

You can in/exclude filters by class name by configuring classExp. For example:
```php docblock
* @ApiFilter(FilterLogic::class, arguments={"classExp"="/ApiPlatform\\Core\\Bridge\\Doctrine\\Orm\\Filter\\+/"})
```
will only apply API Platform ORM Filters in logic context.

Installation
------------
With composer:
Expand All @@ -61,38 +67,80 @@ Then add the bundle to your api config/bundles.php:
];
```

Nested properties workaround
----------------------------
The built-in filters of Api Platform normally generate INNER JOINs. As a result
combining them with OR may not produce results as expected for properties
nested over nullable and to many associations, , see [this issue](https://github.com/metaclass-nl/filter-bundle/issues/2).

As a workaround FilterLogic can convert all inner joins into left joins:
```php8
#[ApiResource]
#[ApiFilter(SearchFilter::class, properties: ['title' => 'partial', 'keywords.word' => 'exact'])]
#[ApiFilter(FilterLogic::class, arguments: ['innerJoinsLeft' => true])]
class Article {
// ...
}
```

In case you do not like FilterLogic messing with the joins you can make
the built-in filters of Api Platform generate left joins themselves by first adding
a left join and removing it later:
```php8
#[ApiResource]
#[ApiFilter(AddFakeLeftJoin::class)]
#[ApiFilter(SearchFilter::class, properties: ['title' => 'partial', 'keywords.word' => 'exact'])]
#[ApiFilter(FilterLogic::class)]
#[ApiFilter(RemoveFakeLeftJoin::class)]
class Article {
// ...
}
```

With one fo these workarounds the following will find Articles whose title contains 'pro'
as well as those whose keywords contain one whose word is 'php'.
```uri
/articles/?or[title]=pro&or[keywords.word]=php
```
Without a workaround Articles without any keywords will not be found,
even if their titles contain 'pro'.

Both workarounds do change the behavior of ExistsFilter =false with nested properties.
Normally this filter only finds entities that reference at least one entity
whose nested property contains NULL, but with left joins it will also find entities
whose reference itself is empty or NULL. This does break backward compatibility.
This can be solved by extending ExistsFilter, but that is not included
in this Bundle because IMHO the old behavior is not like one would expect given
the semantics of "exists" and therefore should be considered a bug unless it is
documented explicitly to be intentional.

Limitations
-----------
Combining filters through OR and nested logic may be a harder task for your
database and require different indexes. Except for small tables performance
testing and analysis is advisable prior to deployment.

Only works with filters implementing ContextAwareFilterInterface, other filters
are ignoored. Works with built in filters of Api Platform, except for DateFilter
Works with built in filters of Api Platform, except for DateFilter
with EXCLUDE_NULL. A DateFilter subclass is provided to correct this. However,
the built in filters of Api Platform contain a bug with respect to the JOINs
they generate, see [this issue](https://github.com/metaclass-nl/filter-bundle/issues/2).
the built in filters of Api Platform IMHO contain a bug with respect to the JOINs
they generate.
As a result, combining them with OR does not work as expected with properties
nested over to-many and nullable associations.
nested over to-many and nullable associations. Workarounds are provided, but they
do change the behavior of ExistsFilter =false.

Assumes that filters create semantically complete expressions in the sense that
expressions added to the QueryBundle through ::andWhere or ::orWhere do not depend
on one another so that the intended logic is not compromised if they are recombined
with the others by either Doctrine\ORM\Query\Expr\Andx or Doctrine\ORM\Query\Expr\Orx.

May Fail if a filter uses QueryBuilder::where or ::add.
Only works with filters implementing ContextAwareFilterInterface, other filters
are ignoored.

You are advised to check the code of all custom and third party Filters and
May Fail if a filter uses QueryBuilder::where or ::add. You are advised to check the code of all custom and third party Filters and
not to combine those that use QueryBuilder::where or ::add with FilterLogic
or that produce complex logic that is not semantically complete. For an
example of semantically complete and incomplete expressions see [DateFilterTest](./tests/Filter/DateFilterTest.php).

You can in/exclude filters by class name by configuring classExp. For example:
```php docblock
* @ApiFilter(FilterLogic::class, arguments={"classExp"="/ApiPlatform\\Core\\Bridge\\Doctrine\\Orm\\Filter\\+/"})
```
will only apply API Platform ORM Filters in logic context.

Credits and License
-------------------
Copyright (c) [MetaClass](https://www.metaclass.nl/), Groningen, 2021. [MetaClass](https://www.metaclass.nl/) offers software development and support in the Netherlands for Symfony, API Platform, React.js and Next.js
Expand Down
13 changes: 13 additions & 0 deletions src/Entity/TestEntity.php
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

namespace Metaclass\FilterBundle\Entity;

use Doctrine\Common\Collections\Collection;
use Doctrine\ORM\Mapping as ORM;

/**
Expand Down Expand Up @@ -45,4 +46,16 @@ class TestEntity
* @ORM\Column(type="boolean", nullable=true)
*/
public $bool;

/**
* @var TestRelated
* @ORM\ManyToOne(targetEntity="Metaclass\FilterBundle\Entity\TestRelated", inversedBy="toMany")
*/
public $toOneNullable;

/**
* @var Collection
* @ORM\OneToMany(targetEntity="Metaclass\FilterBundle\Entity\TestRelated", mappedBy="project")
*/
public $toMany;
}
54 changes: 54 additions & 0 deletions src/Entity/TestRelated.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
<?php


namespace Metaclass\FilterBundle\Entity;

use Doctrine\ORM\Mapping as ORM;

/**
* Class TestRelated
* @package Metaclass\FilterBundle\Entity
* @ORM\Entity
*/
class TestRelated
{
/**
* @var int The entity Id
*
* @ORM\Id
* @ORM\GeneratedValue
* @ORM\Column(type="integer")
*/
public $id = 0;

/**
* @var float
* @ORM\Column(type="float")
*/
public $numb = 0.0;

/**
* @var string
* @ORM\Column
*/
public $text;

/**
* @var \DateTime|null
* @ORM\Column(type="date", nullable=true)
*/
public $dd;

/**
* @var bool|null
* @ORM\Column(type="boolean", nullable=true)
*/
public $bool;

/**
* @var TestEntity
* @ORM\ManyToOne(targetEntity="Metaclass\FilterBundle\Entity\TestEntity", inversedBy="toMany")
* @ORM\JoinColumn(referencedColumnName="id", nullable=false)
*/
public $testEntity;
}
47 changes: 47 additions & 0 deletions src/Filter/AddFakeLeftJoin.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
<?php

namespace Metaclass\FilterBundle\Filter;

use ApiPlatform\Core\Bridge\Doctrine\Orm\Filter\ContextAwareFilterInterface;
use ApiPlatform\Core\Bridge\Doctrine\Orm\Util\QueryNameGeneratorInterface;
use Doctrine\ORM\QueryBuilder;
use Doctrine\ORM\Query\Expr\Join;

/**
* Filter to be added as the first ApiFilter of an ApiResource.
* Adds a Fake Left Join to the QueryBuilder so that the filters that come
* afterwards that use \ApiPlatform\Core\Bridge\Doctrine\Orm\Util\QueryBuilderHelper::addJoinOnce
* will use Left Joins instead of Inner Joins.
* WARNING: This changes the behavior of ExistsFilter =false, consider
* using ExistFilter included in this bundle instead.
*/
class AddFakeLeftJoin implements ContextAwareFilterInterface
{
public static $FAKEJOIN = "Fake.kdoejfndnsklslwkweofdjhsd";

/** {@inheritdoc } */
public function getDescription(string $resourceClass): array
{
// No description
return [];
}

/**
* {@inheritdoc}
*/
public function apply(
QueryBuilder $queryBuilder,
QueryNameGeneratorInterface $queryNameGenerator,
string $resourceClass,
string $operationName = null,
array $context = []
) {
// $queryBuilder->leftJoin(self::$FAKEJOIN, null);
$rootAliases = $queryBuilder->getRootAliases();
$join = new Join(
Join::LEFT_JOIN,
self::$FAKEJOIN
);
$queryBuilder->add('join', [$rootAliases[0] => $join], true);
}
}
50 changes: 47 additions & 3 deletions src/Filter/FilterLogic.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
use ApiPlatform\Core\Api\FilterCollection;
use ApiPlatform\Core\Bridge\Doctrine\Orm\Filter\AbstractContextAwareFilter;
use ApiPlatform\Core\Bridge\Doctrine\Orm\Filter\ContextAwareFilterInterface;
use ApiPlatform\Core\Bridge\Doctrine\Orm\Filter\FilterInterface;
use ApiPlatform\Core\Bridge\Doctrine\Orm\Filter\OrderFilter;
use ApiPlatform\Core\Bridge\Doctrine\Orm\Util\QueryNameGeneratorInterface;
use ApiPlatform\Core\Metadata\Resource\Factory\ResourceMetadataFactoryInterface;
Expand All @@ -15,12 +14,16 @@
use Doctrine\Persistence\ManagerRegistry;
use Psr\Container\ContainerInterface;
use Psr\Log\LoggerInterface;
use Symfony\Component\HttpFoundation\RequestStack;
use Symfony\Component\Serializer\NameConverter\NameConverterInterface;
use Doctrine\ORM\Query\Expr\Join;

/**
* Combines existing API Platform ORM Filters with AND and OR.
* For usage and limitations see https://gist.github.com/metaclass-nl/790a5c8e9064f031db7d3379cc47c794
* WARNING: $innerJoinsLeft=true changes the behavior of ExistsFilter =false,
* and though it makes it more like one would expect given the semantics of its name,
* it does break backward compatibility.
*
* Copyright (c) MetaClass, Groningen, 2021. MIT License
*/
class FilterLogic extends AbstractContextAwareFilter
Expand All @@ -33,19 +36,25 @@ class FilterLogic extends AbstractContextAwareFilter
private $classExp;
/** @var ContextAwareFilterInterface[] */
private $filters;
/** @var bool Wheather to replace all inner joins by left joins */
private $innerJoinsLeft;

/**
* @param ResourceMetadataFactoryInterface $resourceMetadataFactory
* @param ContainerInterface|FilterCollection $filterLocator
* @param $regExp string Filter classes must match this to be applied with logic
* @param $innerJoinsLeft bool Wheather to replace all inner joins by left joins.
* This makes the standard Api Platform filters combine properly with OR,
* but also changes the behavior of ExistsFilter =false.
* {@inheritdoc}
*/
public function __construct(ResourceMetadataFactoryInterface $resourceMetadataFactory, $filterLocator, ManagerRegistry $managerRegistry, LoggerInterface $logger = null, array $properties = null, NameConverterInterface $nameConverter = null, string $classExp='//')
public function __construct(ResourceMetadataFactoryInterface $resourceMetadataFactory, $filterLocator, ManagerRegistry $managerRegistry, LoggerInterface $logger = null, array $properties = null, NameConverterInterface $nameConverter = null, string $classExp='//', $innerJoinsLeft=false)
{
parent::__construct($managerRegistry, null, $logger, $properties, $nameConverter);
$this->resourceMetadataFactory = $resourceMetadataFactory;
$this->filterLocator = $filterLocator;
$this->classExp = $classExp;
$this->innerJoinsLeft = $innerJoinsLeft;
}

/** {@inheritdoc } */
Expand All @@ -65,6 +74,7 @@ public function apply(QueryBuilder $queryBuilder, QueryNameGeneratorInterface $q
if (!isset($context['filters']) || !\is_array($context['filters'])) {
throw new \InvalidArgumentException('::apply without $context[filters] not supported');
}

$this->filters = $this->getFilters($resourceClass, $operationName);

if (isset($context['filters']['and']) ) {
Expand All @@ -86,6 +96,10 @@ public function apply(QueryBuilder $queryBuilder, QueryNameGeneratorInterface $q
$queryBuilder->orWhere($exp);
};
}

if ($this->innerJoinsLeft) {
$this->replaceInnerJoinsByLeftJoins($queryBuilder);
}
}

/**
Expand Down Expand Up @@ -258,4 +272,34 @@ protected function getFilters($resourceClass, $operationName)
}
return $result;
}

/**
* The filters that come standard with Api Platform create inner joins,
* but for nullable and to many references we need Left Joins for OR
* to also produce results that are not related.
* WARNING: This changes the behavior of ExistsFilter =false, consider
* using ExistFilter included in this bundle instead.
* @param QueryBuilder $queryBuilder
*/
protected function replaceInnerJoinsByLeftJoins(QueryBuilder $queryBuilder) {
$joinPart = $queryBuilder->getDQLPart('join');
$result = [];
foreach ($joinPart as $rootAlias => $joins) {
foreach ($joins as $i => $joinExp) {
if (Join::INNER_JOIN === $joinExp->getJoinType()) {
$result[$rootAlias][$i] = new Join(
Join::LEFT_JOIN,
$joinExp->getJoin(),
$joinExp->getAlias(),
$joinExp->getConditionType(),
$joinExp->getCondition(),
$joinExp->getIndexBy()
);
} else {
$result[$rootAlias][$i] = $joinExp;
}
}
}
$queryBuilder->add('join', $result);
}
}
Loading

0 comments on commit d48969d

Please sign in to comment.