#15 when no where filter is used, do not replace inner joins by left …

…joins - ::apply now checks if any logic is applied #14 when no where filter is used, don't apply - added check for empty or
metaclass-nl · Sep 20, 2022 · b18e410 · b18e410
1 parent 9818955
commit b18e410
Show file tree

Hide file tree

Showing 2 changed files with 82 additions and 8 deletions.
diff --git a/src/Filter/FilterLogic.php b/src/Filter/FilterLogic.php
@@ -77,26 +77,32 @@ public function apply(QueryBuilder $queryBuilder, QueryNameGeneratorInterface $q
 
         $this->filters = $this->getFilters($resourceClass, $operationName);
 
+        $logic = false; #15 when no where filter is used, do not replace inner joins by left joins
         if (isset($context['filters']['and']) ) {
             $expressions = $this->filterProperty('and', $context['filters']['and'], $queryBuilder, $queryNameGenerator, $resourceClass, $operationName, $context);
             foreach($expressions as $exp) {
                 $queryBuilder->andWhere($exp);
+                $logic = true;
             };
         }
         if (isset($context['filters']['not']) ) {
             // NOT expressions are combined by parent logic, here defaulted to AND
             $expressions = $this->filterProperty('not', $context['filters']['not'], $queryBuilder, $queryNameGenerator, $resourceClass, $operationName, $context);
             foreach($expressions as $exp) {
                 $queryBuilder->andWhere(new Expr\Func('NOT', [$exp]));
+                $logic = true;
             };
         }
         #Issue 10: for security allways AND with existing criteria
         if (isset($context['filters']['or'])) {
             $expressions = $this->filterProperty('or', $context['filters']['or'], $queryBuilder, $queryNameGenerator, $resourceClass, $operationName, $context);
-            $queryBuilder->andWhere(new Expr\Orx($expressions));
+            if (!empty($expressions)) {
+                $queryBuilder->andWhere(new Expr\Orx($expressions));
+                $logic = true;
+            }
         }
 
-        if ($this->innerJoinsLeft) {
+        if ($this->innerJoinsLeft && $logic) {
             $this->replaceInnerJoinsByLeftJoins($queryBuilder);
         }
     }
@@ -130,6 +136,9 @@ public function generateExpressions(QueryBuilder $queryBuilder, QueryNameGenerat
      */
     protected function doGenerate($queryBuilder, $queryNameGenerator, $resourceClass, $operationName, $context)
     {
+        if (empty($context['filters'])) {
+            return;
+        }
         $oldWhere = $queryBuilder->getDQLPart('where');
 
         // replace by marker expression
@@ -293,7 +302,7 @@ protected function replaceInnerJoinsByLeftJoins(QueryBuilder $queryBuilder) {
                         $joinExp->getConditionType(),
                         $joinExp->getCondition(),
                         $joinExp->getIndexBy()
-                      );
+                    );
                 } else {
                     $result[$rootAlias][$i] = $joinExp;
                 }

diff --git a/tests/Filter/FilterLogicWithAnnotationTest.php b/tests/Filter/FilterLogicWithAnnotationTest.php
@@ -56,6 +56,40 @@ public function setUp(): void
         self::assertNotNull($this->filterLogic, "this->filterLogic");
     }
 
+    public function testNoLogic()
+    {
+        $reqData = null;
+        parse_str('', $reqData);
+        // var_dump($reqData);
+        $context = ['filters' => $reqData];
+        foreach ($this->filters as $filter) {
+            $filter->apply($this->testEntityQb, $this->queryNameGen, TestEntity::class, 'get', $context);
+        }
+
+        $this->assertEquals(
+            str_replace('
+', '', "SELECT o FROM Metaclass\FilterBundle\Entity\TestEntity o"),
+            $this->testEntityQb->getDQL(),
+            'DQL');
+    }
+
+    public function testOrNoFilter()
+    {
+        $reqData = null;
+        parse_str('or', $reqData);
+        // var_dump($reqData);
+        $context = ['filters' => $reqData];
+        foreach ($this->filters as $filter) {
+            $filter->apply($this->testEntityQb, $this->queryNameGen, TestEntity::class, 'get', $context);
+        }
+
+        $this->assertEquals(
+            str_replace('
+', '', "SELECT o FROM Metaclass\FilterBundle\Entity\TestEntity o"),
+            $this->testEntityQb->getDQL(),
+            'DQL');
+    }
+
     public function testDdFilterAnd()
     {
         $reqData = null;
@@ -135,11 +169,11 @@ public function testRexExp()
             'Parameter dd_p1');
     }
 
-    public function testInnerJoinsLeft()
+    public function testInnerJoinsLeftDdFilterOr()
     {
         $this->assertTrue(Reflection::getProperty($this->filterLogic, 'innerJoinsLeft'));
         $reqData = null;
-        parse_str('exists[toMany.bool]=false', $reqData);
+        parse_str('exists[toMany.bool]=false&or[dd][before]=2010-02-02', $reqData);
         $context = ['filters' => $reqData];
         foreach ($this->filters as $filter) {
             $filter->apply($this->testEntityQb, $this->queryNameGen, TestEntity::class, 'get', $context);
@@ -149,14 +183,19 @@ public function testInnerJoinsLeft()
             str_replace('
 ', '', "SELECT o FROM Metaclass\FilterBundle\Entity\TestEntity o 
 LEFT JOIN o.toMany toMany_a1 
-WHERE toMany_a1.bool IS NULL"),
+WHERE toMany_a1.bool IS NULL 
+AND (o.dd <= :dd_p1 AND o.dd IS NOT NULL)"),
             $this->testEntityQb->getDQL(),
             'DQL');
+        $this->assertEquals(
+            '2010-02-02',
+            $this->testEntityQb->getParameter('dd_p1')->getValue()->format('Y-m-d'),
+            'Parameter dd_p1');
     }
 
-    public function testNoInnerJoinsLeft()
+    public function testInnerJoinsLeftNoLogic()
     {
-        Reflection::setProperty($this->filterLogic, 'innerJoinsLeft', false);
+        $this->assertTrue(Reflection::getProperty($this->filterLogic, 'innerJoinsLeft'));
         $reqData = null;
         parse_str('exists[toMany.bool]=false', $reqData);
         $context = ['filters' => $reqData];
@@ -173,6 +212,32 @@ public function testNoInnerJoinsLeft()
             'DQL');
     }
 
+
+    public function testNoInnerJoinsLeftDdFilterOr()
+    {
+        Reflection::setProperty($this->filterLogic, 'innerJoinsLeft', false);
+        $reqData = null;
+        parse_str('exists[toMany.bool]=false&or[dd][before]=2010-02-02', $reqData);
+        $context = ['filters' => $reqData];
+        foreach ($this->filters as $filter) {
+            $filter->apply($this->testEntityQb, $this->queryNameGen, TestEntity::class, 'get', $context);
+        }
+
+        $this->assertEquals(
+            str_replace('
+', '', "SELECT o FROM Metaclass\FilterBundle\Entity\TestEntity o 
+INNER JOIN o.toMany toMany_a1 
+WHERE toMany_a1.bool IS NULL 
+AND (o.dd <= :dd_p1 AND o.dd IS NOT NULL)"),
+            $this->testEntityQb->getDQL(),
+            'DQL');
+        $this->assertEquals(
+            '2010-02-02',
+            $this->testEntityQb->getParameter('dd_p1')->getValue()->format('Y-m-d'),
+            'Parameter dd_p1');
+
+    }
+
     public function testWithFakeLeftJoin()
     {
         Reflection::setProperty($this->filterLogic, 'innerJoinsLeft', false);