performance tuning on timesheets (#874)

src/API/TimesheetController.php

@@ -198,7 +198,7 @@ class TimesheetController extends BaseApiController
         }
 
         /** @var Pagerfanta $data */
-        $data = $this->repository->findByQuery($query);
+        $data = $this->repository->getPagerfantaForQuery($query);
         $data = (array) $data->getCurrentPageResults();
 
         $view = new View($data, 200);

src/Controller/ExportController.php

@@ -143,13 +143,10 @@ class ExportController extends AbstractController
      */
     protected function getEntries(ExportQuery $query)
     {
-        $query->setResultType(ExportQuery::RESULT_TYPE_QUERYBUILDER);
         $query->getBegin()->setTime(0, 0, 0);
         $query->getEnd()->setTime(23, 59, 59);
 
-        $queryBuilder = $this->timesheetRepository->findByQuery($query);
+        return $this->timesheetRepository->getTimesheetsForQuery($query);
-
-        return $queryBuilder->getQuery()->getResult();
     }
 
     /**

src/Controller/InvoiceController.php

@@ -18,7 +18,6 @@ use App\Model\InvoiceModel;
 use App\Repository\InvoiceTemplateRepository;
 use App\Repository\Query\BaseQuery;
 use App\Repository\Query\InvoiceQuery;
-use App\Repository\Query\TimesheetQuery;
 use App\Repository\TimesheetRepository;
 use App\Timesheet\UserDateTimeFactory;
 use Sensio\Bundle\FrameworkExtraBundle\Configuration\Security;
@@ -166,8 +165,6 @@ class InvoiceController extends AbstractController
             return [];
         }
 
-        $query->setResultType(TimesheetQuery::RESULT_TYPE_QUERYBUILDER);
-
         if (null === $query->getBegin()) {
             $query->setBegin($this->dateTimeFactory->createDateTime('first day of this month'));
         }
@@ -177,9 +174,7 @@ class InvoiceController extends AbstractController
         $query->getBegin()->setTime(0, 0, 0);
         $query->getEnd()->setTime(23, 59, 59);
 
-        $queryBuilder = $repository->findByQuery($query);
+        return $repository->getTimesheetsForQuery($query);
-
-        return $queryBuilder->getQuery()->getResult();
     }
 
     /**

src/Controller/TimesheetAbstractController.php

@@ -23,7 +23,6 @@ use App\Timesheet\TrackingMode\TrackingModeInterface;
 use App\Timesheet\TrackingModeService;
 use App\Timesheet\UserDateTimeFactory;
 use Doctrine\Common\Collections\ArrayCollection;
-use Pagerfanta\Pagerfanta;
 use Symfony\Component\Form\FormInterface;
 use Symfony\Component\HttpFoundation\Request;
 use Symfony\Component\HttpFoundation\Response;
@@ -107,11 +106,10 @@ abstract class TimesheetAbstractController extends AbstractController
             );
         }
 
-        /* @var $entries Pagerfanta */
+        $pager = $this->getRepository()->getPagerfantaForQuery($query);
-        $entries = $this->getRepository()->findByQuery($query);
 
         return $this->render($renderTemplate, [
-            'entries' => $entries,
+            'entries' => $pager,
             'page' => $query->getPage(),
             'query' => $query,
             'showFilter' => $form->isSubmitted(),
@@ -212,7 +210,6 @@ abstract class TimesheetAbstractController extends AbstractController
     protected function export(Request $request, string $renderTemplate)
     {
         $query = new TimesheetQuery();
-        $query->setResultType(TimesheetQuery::RESULT_TYPE_OBJECTS);
 
         $form = $this->getToolbarForm($query);
         $form->handleRequest($request);
@@ -238,8 +235,7 @@ abstract class TimesheetAbstractController extends AbstractController
             $query->setUser($this->getUser());
         }
 
-        /* @var $entries Pagerfanta */
+        $entries = $this->getRepository()->getTimesheetsForQuery($query);
-        $entries = $this->getRepository()->findByQuery($query);
 
         return $this->render($renderTemplate, [
             'entries' => $entries,

src/Repository/Loader/LoaderInterface.php

@@ -0,0 +1,20 @@
+<?php
+
+/*
+ * This file is part of the Kimai time-tracking app.
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace App\Repository\Loader;
+
+interface LoaderInterface
+{
+    /**
+     * Prepares the given database results, so no lazy loading will be performed.
+     *
+     * @param array $results
+     */
+    public function loadResults(array $results): void;
+}

src/Repository/Loader/TimesheetIdLoader.php

@@ -0,0 +1,95 @@
+<?php
+
+/*
+ * This file is part of the Kimai time-tracking app.
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace App\Repository\Loader;
+
+use App\Entity\Project;
+use App\Entity\Timesheet;
+use Doctrine\ORM\EntityManagerInterface;
+
+final class TimesheetIdLoader implements LoaderInterface
+{
+    /**
+     * @var EntityManagerInterface
+     */
+    private $entityManager;
+
+    public function __construct(EntityManagerInterface $entityManager)
+    {
+        $this->entityManager = $entityManager;
+    }
+
+    /**
+     * @param int[] $ids
+     */
+    public function loadResults(array $ids): void
+    {
+        if (empty($ids)) {
+            return;
+        }
+
+        $em = $this->entityManager;
+
+        $qb = $em->createQueryBuilder();
+        $projects = $qb->select('PARTIAL t.{id}', 'project')
+            ->from(Timesheet::class, 't')
+            ->leftJoin('t.project', 'project')
+            ->andWhere($qb->expr()->in('t.id', $ids))
+            ->getQuery()
+            ->execute();
+
+        if (!empty($projects)) {
+            $projectIds = array_map(function (Timesheet $timesheet) {
+                return $timesheet->getProject()->getId();
+            }, $projects);
+
+            $qb = $em->createQueryBuilder();
+            $qb->select('PARTIAL p.{id}', 'customer')
+                ->from(Project::class, 'p')
+                ->leftJoin('p.customer', 'customer')
+                ->andWhere($qb->expr()->in('p.id', $projectIds))
+                ->getQuery()
+                ->execute();
+        }
+
+        $qb = $em->createQueryBuilder();
+        $qb->select('PARTIAL t.{id}', 'activity')
+            ->from(Timesheet::class, 't')
+            ->leftJoin('t.activity', 'activity')
+            ->andWhere($qb->expr()->in('t.id', $ids))
+            ->getQuery()
+            ->execute();
+
+        $qb = $em->createQueryBuilder();
+        $qb->select('PARTIAL t.{id}', 'user')
+            ->from(Timesheet::class, 't')
+            ->leftJoin('t.user', 'user')
+            ->andWhere($qb->expr()->in('t.id', $ids))
+            ->getQuery()
+            ->execute();
+
+        $qb = $em->createQueryBuilder();
+        $qb->select('PARTIAL t.{id}', 'tags')
+            ->from(Timesheet::class, 't')
+            ->leftJoin('t.tags', 'tags')
+            ->andWhere($qb->expr()->in('t.id', $ids))
+            ->getQuery()
+            ->execute();
+
+        /*
+        $qb = $em->createQueryBuilder();
+        $qb->select('PARTIAL t.{id}', 'meta')
+            ->from(Timesheet::class, 't')
+            ->leftJoin('t.meta', 'meta')
+            ->andWhere($qb->expr()->in('t.id', $ids))
+            ->getQuery()
+            ->execute();
+        */
+    }
+}

src/Repository/Loader/TimesheetLoader.php

@@ -0,0 +1,38 @@
+<?php
+
+/*
+ * This file is part of the Kimai time-tracking app.
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace App\Repository\Loader;
+
+use App\Entity\Timesheet;
+use Doctrine\ORM\EntityManagerInterface;
+
+final class TimesheetLoader implements LoaderInterface
+{
+    /**
+     * @var TimesheetIdLoader
+     */
+    private $loader;
+
+    public function __construct(EntityManagerInterface $entityManager)
+    {
+        $this->loader = new TimesheetIdLoader($entityManager);
+    }
+
+    /**
+     * @param Timesheet[] $timesheets
+     */
+    public function loadResults(array $timesheets): void
+    {
+        $ids = array_map(function (Timesheet $timesheet) {
+            return $timesheet->getId();
+        }, $timesheets);
+
+        $this->loader->loadResults($ids);
+    }
+}

src/Repository/Paginator/TimesheetPaginator.php

@@ -0,0 +1,73 @@
+<?php
+
+/*
+ * This file is part of the Kimai time-tracking app.
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace App\Repository\Paginator;
+
+use App\Repository\Loader\TimesheetLoader;
+use Doctrine\ORM\Query;
+use Doctrine\ORM\QueryBuilder;
+use Pagerfanta\Adapter\AdapterInterface;
+
+final class TimesheetPaginator implements AdapterInterface
+{
+    /**
+     * @var QueryBuilder
+     */
+    private $query;
+    /**
+     * @var int
+     */
+    private $results = 0;
+    /**
+     * @var TimesheetLoader
+     */
+    private $loader;
+
+    public function __construct(QueryBuilder $query, int $results)
+    {
+        $this->query = $query;
+        $this->results = $results;
+        $this->loader = new TimesheetLoader($query->getEntityManager());
+    }
+
+    /**
+     * {@inheritdoc}
+     */
+    public function getNbResults()
+    {
+        return $this->results;
+    }
+
+    private function getResults(Query $query)
+    {
+        $results = $query->execute();
+
+        $this->loader->loadResults($results);
+
+        return $results;
+    }
+
+    /**
+     * {@inheritdoc}
+     */
+    public function getSlice($offset, $length)
+    {
+        $query = $this->query
+            ->getQuery()
+            ->setFirstResult($offset)
+            ->setMaxResults($length);
+
+        return $this->getResults($query);
+    }
+
+    public function getAll()
+    {
+        return $this->getResults($this->query->getQuery());
+    }
+}

src/Repository/TimesheetRepository.php

@@ -16,13 +16,16 @@ use App\Model\Statistic\Day;
 use App\Model\Statistic\Month;
 use App\Model\Statistic\Year;
 use App\Model\TimesheetStatistic;
+use App\Repository\Loader\TimesheetLoader;
+use App\Repository\Paginator\TimesheetPaginator;
 use App\Repository\Query\TimesheetQuery;
 use DateTime;
 use Doctrine\DBAL\Types\Type;
+use Doctrine\ORM\EntityRepository;
 use Doctrine\ORM\QueryBuilder;
 use Pagerfanta\Pagerfanta;
 
-class TimesheetRepository extends AbstractRepository
+class TimesheetRepository extends EntityRepository
 {
     public const STATS_QUERY_DURATION = 'duration';
     public const STATS_QUERY_RATE = 'rate';
@@ -352,7 +355,7 @@ class TimesheetRepository extends AbstractRepository
             ->join('t.project', 'p')
             ->join('p.customer', 'c')
             ->leftJoin('t.tags', 'tags')
-            ->where($qb->expr()->isNotNull('t.begin'))
+            ->andWhere($qb->expr()->isNotNull('t.begin'))
             ->andWhere($qb->expr()->isNull('t.end'))
             ->orderBy('t.begin', 'DESC');
 
@@ -402,22 +405,45 @@ class TimesheetRepository extends AbstractRepository
         return $counter;
     }
 
+    public function getPagerfantaForQuery(TimesheetQuery $query): Pagerfanta
+    {
+        $paginator = new Pagerfanta($this->getPaginatorForQuery($query));
+        $paginator->setMaxPerPage($query->getPageSize());
+        $paginator->setCurrentPage($query->getPage());
+
+        return $paginator;
+    }
+
+    protected function getPaginatorForQuery(TimesheetQuery $query): TimesheetPaginator
+    {
+        $qb = $this->getQueryBuilderForQuery($query);
+        $qb->select($qb->expr()->countDistinct('t.id'))->resetDQLPart('orderBy');
+        $counter = (int) $qb->getQuery()->getSingleScalarResult();
+
+        $qb = $this->getQueryBuilderForQuery($query);
+        $qb->select('t');
+
+        $paginator = new TimesheetPaginator($qb, $counter);
+
+        return $paginator;
+    }
+
     /**
      * @param TimesheetQuery $query
-     * @return QueryBuilder|Pagerfanta|array
+     * @return Timesheet[]
      */
-    public function findByQuery(TimesheetQuery $query)
+    public function getTimesheetsForQuery(TimesheetQuery $query): array
+    {
+        $paginator = $this->getPaginatorForQuery($query);
+
+        return $paginator->getAll();
+    }
+
+    protected function getQueryBuilderForQuery(TimesheetQuery $query): QueryBuilder
     {
         $qb = $this->getEntityManager()->createQueryBuilder();
 
-        $qb->select('t', 'a', 'p', 'c', 'u', 'tags')
+        $qb->from(Timesheet::class, 't');
-            ->from(Timesheet::class, 't')
-            ->leftJoin('t.activity', 'a')
-            ->leftJoin('t.user', 'u')
-            ->leftJoin('t.project', 'p')
-            ->leftJoin('p.customer', 'c')
-            ->leftJoin('t.tags', 'tags')
-            ->orderBy('t.' . $query->getOrderBy(), $query->getOrder());
 
         if (null !== $query->getUser()) {
             $qb->andWhere('t.user = :user')
@@ -456,6 +482,7 @@ class TimesheetRepository extends AbstractRepository
                 $qb->andWhere('t.project = :project')
                     ->setParameter('project', $query->getProject());
             } elseif (null !== $query->getCustomer()) {
+                $qb->join('t.project', 'p');
                 $qb->andWhere('p.customer = :customer')
                     ->setParameter('customer', $query->getCustomer());
             }
@@ -467,7 +494,9 @@ class TimesheetRepository extends AbstractRepository
                 ->setParameter('tags', $query->getTags());
         }
 
-        return $this->getBaseQueryResult($qb, $query);
+        $qb->orderBy('t.' . $query->getOrderBy(), $query->getOrder());
+
+        return $qb;
     }
 
     /**
@@ -516,16 +545,17 @@ class TimesheetRepository extends AbstractRepository
         $ids = array_column($results, 'maxid');
 
         $qb = $this->getEntityManager()->createQueryBuilder();
-        $qb->select('t', 'a', 'p', 'c', 'tags')
+        $qb->select('t')
             ->from(Timesheet::class, 't')
-            ->join('t.activity', 'a')
-            ->join('t.project', 'p')
-            ->join('p.customer', 'c')
-            ->leftJoin('t.tags', 'tags')
             ->andWhere($qb->expr()->in('t.id', $ids))
             ->orderBy('t.end', 'DESC')
         ;
 
-        return $qb->getQuery()->getResult();
+        $results = $qb->getQuery()->getResult();
+
+        $loader = new TimesheetLoader($qb->getEntityManager());
+        $loader->loadResults($results);
+
+        return $results;
     }
 }

templates/export/renderer/default.html.twig

@@ -8,6 +8,7 @@
     'project': true,
     'activity': true,
     'description': false,
+    'tags': false,
     'exported': false,
     'hourlyRate': false,
     'fixedRate': false,
@@ -267,6 +268,11 @@
                                 {{ entry.description|desc2html }}
                             {% endif %}
                         </td>
+                        <td class="column-tags" {% if not columns.tags %}style="display: none"{% endif %}>
+                            {% if entry.tags is not empty %}
+                                {{ entry.tagsAsArray|join(', ') }}
+                            {% endif %}
+                        </td>
                         <td class="column-exported" {% if not columns.exported %}style="display: none"{% endif %}>
                             {% if entry.exported %}
                                 {{ 'entryState.exported'|trans }}

tests/Repository/TimesheetRepositoryTest.php

@@ -21,7 +21,6 @@ use App\Repository\Query\TimesheetQuery;
 use App\Repository\RepositoryException;
 use App\Repository\TimesheetRepository;
 use App\Tests\DataFixtures\TimesheetFixtures;
-use Doctrine\ORM\QueryBuilder;
 use Pagerfanta\Pagerfanta;
 
 /**
@@ -38,19 +37,10 @@ class TimesheetRepositoryTest extends AbstractRepositoryTest
 
         $query = new TimesheetQuery();
 
-        $result = $repository->findByQuery($query);
+        $result = $repository->getPagerfantaForQuery($query);
         $this->assertInstanceOf(Pagerfanta::class, $result);
 
-        $query->setResultType(BaseQuery::RESULT_TYPE_QUERYBUILDER);
+        $result = $repository->getTimesheetsForQuery($query);
-        $result = $repository->findByQuery($query);
-        $this->assertInstanceOf(QueryBuilder::class, $result);
-
-        $query->setResultType(BaseQuery::RESULT_TYPE_PAGER);
-        $result = $repository->findByQuery($query);
-        $this->assertInstanceOf(Pagerfanta::class, $result);
-
-        $query->setResultType(BaseQuery::RESULT_TYPE_OBJECTS);
-        $result = $repository->findByQuery($query);
         $this->assertIsArray($result);
     }
 
@@ -73,7 +63,7 @@ class TimesheetRepositoryTest extends AbstractRepositoryTest
         $query->setState(TimesheetQuery::STATE_STOPPED);
 
         /** @var array $entities */
-        $entities = $repository->findByQuery($query);
+        $entities = $repository->getTimesheetsForQuery($query);
 
         $this->assertCount(1, $entities);
         $this->assertInstanceOf(Timesheet::class, $entities[0]);