Allow filter strategies to set parameters (for invalidating the Doctrine cache) (Case 142739)

src/DependencyInjection/OnRequestDependencyInjector.php

@@ -7,6 +7,7 @@
 use Symfony\Component\EventDispatcher\EventSubscriberInterface;
 use Symfony\Component\HttpKernel\Event\GetResponseEvent;
 use Symfony\Component\HttpKernel\KernelEvents;
+use Webfactory\VisibilityFilterBundle\Filter\SQLFilterAsParameterCollection;
 use Webfactory\VisibilityFilterBundle\Filter\Strategy\FilterStrategy;
 use Webfactory\VisibilityFilterBundle\Filter\VisibilityColumnConsideringSQLFilter;
 use Webfactory\VisibilityFilterBundle\Filter\VisibilityColumnRetriever;
@@ -62,6 +63,6 @@ public function setUpFilter(GetResponseEvent $event): void
         $visibilityFilter = $filterCollection->getFilter(VisibilityColumnConsideringSQLFilter::NAME);
 
         $visibilityFilter->injectDependencies($this->filterStrategy, $this->visibilityColumnRetriever);
-        $visibilityFilter->setParameter('visibility', $this->filterStrategy->getFilterSql(''));
+        $this->filterStrategy->addParameters(new SQLFilterAsParameterCollection($visibilityFilter));
     }
 }

src/DependencyInjection/services.yaml

@@ -3,6 +3,7 @@ services:
         alias: 'Webfactory\VisibilityFilterBundle\Filter\Strategy\ValueInField'
 
     Webfactory\VisibilityFilterBundle\Filter\Strategy\ValueInField:
+        public: true
         arguments:
             - 'y'
 

src/Filter/ParameterCollection.php

@@ -0,0 +1,41 @@
+<?php
+
+namespace Webfactory\VisibilityFilterBundle\Filter;
+
+use Doctrine\DBAL\Types\Types;
+use InvalidArgumentException;
+
+interface ParameterCollection
+{
+    /**
+     * Sets a parameter that can be used by the filter.
+     *
+     * @param string        $name  Name of the parameter.
+     * @param string        $value Value of the parameter.
+     * @param Types::*|null $type  The parameter type. If specified, the given value will be run through
+     *                             the type conversion of this type. This is usually not needed for
+     *                             strings and numeric types.
+     */
+    public function setParameter(string $name, $value, $type = null): void;
+
+    /**
+     * Gets a parameter to use in a query.
+     *
+     * The function is responsible for the right output escaping to use the
+     * value in a query.
+     *
+     * @param string $name Name of the parameter.
+     *
+     * @return string The SQL escaped parameter to use in a query.
+     *
+     * @throws InvalidArgumentException
+     */
+    public function getParameter(string $name);
+
+    /**
+     * Checks if a parameter was set for the filter.
+     *
+     * @param string $name Name of the parameter.
+     */
+    public function hasParameter(string $name): bool;
+}

src/Filter/SQLFilterAsParameterCollection.php

@@ -0,0 +1,33 @@
+<?php
+
+namespace Webfactory\VisibilityFilterBundle\Filter;
+
+use Doctrine\ORM\Query\Filter\SQLFilter;
+
+class SQLFilterAsParameterCollection implements ParameterCollection
+{
+    /**
+     * @var SQLFilter
+     */
+    private $sqlFilter;
+
+    public function __construct(SQLFilter $sqlFilter)
+    {
+        $this->sqlFilter = $sqlFilter;
+    }
+
+    final public function setParameter($name, $value, $type = null): void
+    {
+        $this->sqlFilter->setParameter($name, $value, $type);
+    }
+
+    final public function getParameter($name)
+    {
+        return $this->sqlFilter->getParameter($name);
+    }
+
+    final public function hasParameter($name): bool
+    {
+        return $this->sqlFilter->hasParameter($name);
+    }
+}

src/Filter/Strategy/FilterStrategy.php

@@ -2,15 +2,26 @@
 
 namespace Webfactory\VisibilityFilterBundle\Filter\Strategy;
 
+use Webfactory\VisibilityFilterBundle\Filter\ParameterCollection;
+
 /**
  * Implement this to apply custom filtering logic to your project.
  */
 interface FilterStrategy
 {
+    /**
+     * Adds all parameters to the collection that affect the resulting filter SQL string. If the filter SQL depends on
+     * factors other than parameters added to the collection, the cache won't be invalidated, which will have undesired
+     * consequences!
+     *
+     * So please add all parameters to the collection.
+     */
+    public function addParameters(ParameterCollection $parameters): void;
+
     /**
      * @param string $visibilityFieldAlias SQL alias for the visibility field of the requested entity
      *
      * @return string A string of SQL that will be included in the WHERE clause to any query requesting an entity with a visibility field
      */
-    public function getFilterSql(string $visibilityFieldAlias): string;
+    public function getFilterSql(string $visibilityFieldAlias, ParameterCollection $parameters): string;
 }

src/Filter/Strategy/ValueInField.php

@@ -2,6 +2,8 @@
 
 namespace Webfactory\VisibilityFilterBundle\Filter\Strategy;
 
+use Webfactory\VisibilityFilterBundle\Filter\ParameterCollection;
+
 /**
  * Filters queries so that only entries with a certain value in their visibility field (@see VisibilityColumn) will
  * be retrieved from the database.
@@ -21,7 +23,12 @@ public function __construct($visibleValue)
         $this->visibleValue = $visibleValue;
     }
 
-    public function getFilterSql(string $visibilityFieldAlias): string
+    public function addParameters(ParameterCollection $parameters): void
+    {
+        // not needed, as $this->visibleValue is immutable
+    }
+
+    public function getFilterSql(string $visibilityFieldAlias, ParameterCollection $parameters): string
     {
         return $visibilityFieldAlias.' = '.$this->getSqlLiteral($this->visibleValue);
     }

src/Filter/VisibilityColumnConsideringSQLFilter.php

@@ -40,7 +40,7 @@ public function addFilterConstraint(ClassMetadata $targetEntity, $targetTableAli
             return ''; // Entity doesn't have visibility information
         }
 
-        return $this->filterStrategy->getFilterSql($targetTableAlias.'.'.$visibilityColumn);
+        return $this->filterStrategy->getFilterSql($targetTableAlias.'.'.$visibilityColumn, new SQLFilterAsParameterCollection($this));
     }
 
     private function assertSetUpCorrectly(): void

tests/DependencyInjection/OnRequestDependencyInjectorTest.php

@@ -10,6 +10,8 @@
 use Symfony\Component\HttpKernel\Event\GetResponseEvent;
 use Symfony\Component\HttpKernel\HttpKernelInterface;
 use Symfony\Component\HttpKernel\KernelEvents;
+use Webfactory\VisibilityFilterBundle\Filter\Strategy\FilterStrategy;
+use Webfactory\VisibilityFilterBundle\Filter\Strategy\ValueInField;
 use Webfactory\VisibilityFilterBundle\Filter\VisibilityColumnConsideringSQLFilter;
 use Webfactory\VisibilityFilterBundle\Tests\Fixtures\Kernel\TestKernel;
 use Webfactory\VisibilityFilterBundle\Tests\Fixtures\VisibilityColumnConsideringSQLFilterMock;
@@ -67,4 +69,18 @@ public function injects_dependencies_into_filter(): void
         $filterMock = $this->entityManager->getFilters()->getFilter(VisibilityColumnConsideringSQLFilter::NAME);
         static::assertTrue($filterMock->haveDependenciesBeenInjected());
     }
+
+    /**
+     * @test
+     */
+    public function calls_addParameters_on_strategy(): void
+    {
+        $strategyMock = $this->createMock(FilterStrategy::class);
+        static::$container->set(ValueInField::class, $strategyMock); // alias can't be overwritten at runtime – so we override the default filter strategy
+
+        $strategyMock->expects($this->once())->method('addParameters');
+
+        $masterRequestEvent = new GetResponseEvent(static::$kernel, new Request(), HttpKernelInterface::MASTER_REQUEST);
+        $this->eventDispatcher->dispatch($masterRequestEvent, KernelEvents::REQUEST);
+    }
 }

tests/Filter/Strategy/ValueInFieldTest.php

@@ -3,6 +3,7 @@
 namespace Webfactory\VisibilityFilterBundle\Tests\Filter\Strategy;
 
 use PHPUnit\Framework\TestCase;
+use Webfactory\VisibilityFilterBundle\Filter\ParameterCollection;
 use Webfactory\VisibilityFilterBundle\Filter\Strategy\ValueInField;
 
 class ValueInFieldTest extends TestCase
@@ -14,7 +15,7 @@ public function gives_correct_SQL_clause_for_string(): void
     {
         $valueInField = new ValueInField('y');
 
-        $sqlClause = $valueInField->getFilterSql('testAlias');
+        $sqlClause = $valueInField->getFilterSql('testAlias', $this->createMock(ParameterCollection::class));
 
         static::assertEquals('testAlias = "y"', $sqlClause);
     }
@@ -26,7 +27,7 @@ public function gives_correct_SQL_clause_for_int(): void
     {
         $valueInField = new ValueInField(2);
 
-        $sqlClause = $valueInField->getFilterSql('testAlias');
+        $sqlClause = $valueInField->getFilterSql('testAlias', $this->createMock(ParameterCollection::class));
 
         static::assertEquals('testAlias = 2', $sqlClause);
     }
@@ -38,7 +39,7 @@ public function gives_correct_SQL_clause_for_false(): void
     {
         $valueInField = new ValueInField(false);
 
-        $sqlClause = $valueInField->getFilterSql('testAlias');
+        $sqlClause = $valueInField->getFilterSql('testAlias', $this->createMock(ParameterCollection::class));
 
         static::assertEquals('testAlias = 0', $sqlClause);
     }
@@ -50,7 +51,7 @@ public function gives_correct_SQL_clause_for_true(): void
     {
         $valueInField = new ValueInField(true);
 
-        $sqlClause = $valueInField->getFilterSql('testAlias');
+        $sqlClause = $valueInField->getFilterSql('testAlias', $this->createMock(ParameterCollection::class));
 
         static::assertEquals('testAlias = 1', $sqlClause);
     }