chore: cleanup

rekalogika · Jul 27, 2024 · c5f2456 · c5f2456
1 parent 0f4e6e3
commit c5f2456
Show file tree

Hide file tree

Showing 3 changed files with 69 additions and 80 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -3,6 +3,7 @@
 # 0.15.2
 
 * refactor: simplify `KeysetExpressionCalculator`
+* chore: cleanup
 
 # 0.15.1
 

diff --git a/packages/rekapager-doctrine-orm-adapter/src/NativeQueryAdapter.php b/packages/rekapager-doctrine-orm-adapter/src/NativeQueryAdapter.php
@@ -13,7 +13,7 @@
 
 namespace Rekalogika\Rekapager\Doctrine\ORM;
 
-use Doctrine\Common\Collections\Criteria;
+use Doctrine\Common\Collections\Expr\Expression;
 use Doctrine\Common\Collections\Order;
 use Doctrine\ORM\EntityManagerInterface;
 use Doctrine\ORM\Query\ResultSetMapping;
@@ -106,22 +106,13 @@ private function verifySQL(
     }
 
     /**
-     * We use Criteria internally only to represent our query parameters
-     *
-     * @param int<0,max> $offset
-     * @param int<1,max> $limit
      * @param null|array<string,mixed> $boundaryValues Key is the property name, value is the bound value. Null if unbounded.
+     * @param non-empty-array<string,Order> $orderings
      */
-    private function getCriteria(
-        int $offset,
-        int $limit,
+    private function generateWhereExpression(
         null|array $boundaryValues,
-        BoundaryType $boundaryType,
-    ): Criteria {
-        $criteria = Criteria::create()
-            ->setFirstResult($offset)
-            ->setMaxResults($limit);
-
+        array $orderings,
+    ): ?Expression {
         // wrap boundary values using QueryParameter
 
         $newBoundaryValues = [];
@@ -133,30 +124,15 @@ private function getCriteria(
 
         $boundaryValues = $newBoundaryValues;
 
-        // if upper bound, reverse the sort order
-
-        if ($boundaryType === BoundaryType::Upper) {
-            $orderings = $this->getReversedSortOrder();
-        } else {
-            $orderings = $this->orderBy;
-        }
-
-        if ($orderings === []) {
-            throw new LogicException('No ordering is set.');
-        }
-
-        $criteria->orderBy($orderings);
-
         // construct the metadata for the next step
 
         $fields = $this->createCalculatorFields($boundaryValues, $orderings);
 
-        if ($fields !== []) {
-            $expression = KeysetExpressionCalculator::calculate($fields);
-            $criteria->where($expression);
+        if ($fields === []) {
+            return null;
         }
 
-        return $criteria;
+        return KeysetExpressionCalculator::calculate($fields);
     }
 
     /**
@@ -192,23 +168,26 @@ private function createCalculatorFields(
     /**
      * @return array<string,Order>
      */
-    private function getReversedSortOrder(): array
+    private function getSortOrder(bool $reverse): array
     {
-        $orderBy = $this->orderBy;
-        $reversed = [];
-
-        foreach ($orderBy as $property => $order) {
-            $reversed[$property] = $order === Order::Ascending ? Order::Descending : Order::Ascending;
+        if (!$reverse) {
+            return $this->orderBy;
         }
 
-        return $reversed;
+        return array_map(
+            static fn (Order $order): Order => $order === Order::Ascending ? Order::Descending : Order::Ascending,
+            $this->orderBy
+        );
     }
 
-    private function generateOrderBy(Criteria $criteria): string
+    /**
+     * @param non-empty-array<string,Order> $orderings
+     */
+    private function generateOrderBy(array $orderings): string
     {
         $orderBy = [];
 
-        foreach ($criteria->orderings() as $field => $order) {
+        foreach ($orderings as $field => $order) {
             $orderBy[] = sprintf('%s %s', $field, $order === Order::Ascending ? 'ASC' : 'DESC');
         }
 
@@ -227,47 +206,53 @@ private function getSQL(
         BoundaryType $boundaryType,
         bool $count = false,
     ): SQLStatement {
-        $criteria = $this->getCriteria(
-            offset: $offset,
-            limit: $limit,
-            boundaryValues: $boundaryValues,
-            boundaryType: $boundaryType
-        );
+        $orderings = $this->getSortOrder($boundaryType === BoundaryType::Upper);
 
-        $orderBy = $this->generateOrderBy($criteria);
+        if ($orderings === []) {
+            throw new LogicException('No ordering is set.');
+        }
 
-        $expression = $criteria->getWhereExpression();
-        $visitor = new KeysetExpressionSQLVisitor();
+        $expression = $this->generateWhereExpression(
+            boundaryValues: $boundaryValues,
+            orderings: $orderings
+        );
 
         if ($expression !== null) {
+            $visitor = new KeysetExpressionSQLVisitor();
             $result = $visitor->dispatch($expression);
             \assert(\is_string($result));
             $where = 'AND ' . $result;
+
+            $parameters = $visitor->getParameters();
         } else {
             $where = '';
+            $parameters = [];
         }
 
+        $orderBy = $this->generateOrderBy($orderings);
+
         $sql = str_replace(
             ['{{SELECT}}', '{{WHERE}}', '{{ORDER}}', '{{LIMIT}}', '{{OFFSET}}'],
             [$this->select, $where, $orderBy, $limit, $offset],
             $count ? $this->countSql : $this->sql
         );
 
-        $parameters = $this->parameters;
+        $sqlParameters = [];
 
-        foreach ($visitor->getParameters() as $template => $parameter) {
-            if (!$parameter instanceof QueryParameter) {
-                throw new UnexpectedValueException('Expected QueryParameter');
+        /** @var mixed $parameter */
+        foreach ([...$parameters, ...$this->parameters] as $template => $parameter) {
+            if ($parameter instanceof QueryParameter) {
+                $sqlParameters[] = new Parameter(
+                    key: $template,
+                    value: $parameter->getValue(),
+                    type: $parameter->getType()
+                );
+            } elseif ($parameter instanceof Parameter) {
+                $sqlParameters[] = $parameter;
             }
-
-            $parameters[] = new Parameter(
-                key: $template,
-                value: $parameter->getValue(),
-                type: $parameter->getType()
-            );
         }
 
-        return new SQLStatement($sql, $parameters);
+        return new SQLStatement($sql, $sqlParameters);
     }
 
     /** @psalm-suppress InvalidReturnType */

diff --git a/packages/rekapager-doctrine-orm-adapter/src/QueryBuilderAdapter.php b/packages/rekapager-doctrine-orm-adapter/src/QueryBuilderAdapter.php
@@ -129,32 +129,35 @@ private function getQueryBuilder(
             $orderings = $this->getSortOrder();
         }
 
-        // adds where expression to the querybuilder
+        // adds the boundary values to the select statement
 
-        $fields = $this->createCalculatorFields($boundaryValues, $orderings);
+        $i = 1;
+        foreach ($this->getBoundaryFieldNames() as $field) {
+            $queryBuilder->addSelect(sprintf('%s AS rekapager_boundary_%s', $field, $i));
+            $i++;
+        }
 
-        if ($fields !== []) {
-            // calculate keyset expression
-            $keysetExpression = KeysetExpressionCalculator::calculate($fields);
+        // returns early if there are no boundary values
 
-            $visitor = new KeysetQueryBuilderVisitor();
-            $queryBuilder->andWhere($visitor->dispatch($keysetExpression));
+        $fields = $this->createCalculatorFields($boundaryValues, $orderings);
 
-            foreach ($visitor->getParameters() as $template => $parameter) {
-                $queryBuilder->setParameter(
-                    $template,
-                    $parameter->getValue(),
-                    $parameter->getType()
-                );
-            }
+        if ($fields === []) {
+            return $queryBuilder;
         }
 
-        // adds the boundary values to the select statement
+        // adds where expression to the querybuilder
 
-        $i = 1;
-        foreach ($this->getBoundaryFieldNames() as $field) {
-            $queryBuilder->addSelect(sprintf('%s AS rekapager_boundary_%s', $field, $i));
-            $i++;
+        $keysetExpression = KeysetExpressionCalculator::calculate($fields);
+
+        $visitor = new KeysetQueryBuilderVisitor();
+        $queryBuilder->andWhere($visitor->dispatch($keysetExpression));
+
+        foreach ($visitor->getParameters() as $template => $parameter) {
+            $queryBuilder->setParameter(
+                $template,
+                $parameter->getValue(),
+                $parameter->getType()
+            );
         }
 
         return $queryBuilder;