Query hints & persistent cache for latest approvals (#5176)

src/Controller/TimesheetTeamController.php

@@ -19,6 +19,7 @@ use App\Form\Model\MultiUserTimesheet;
 use App\Form\TimesheetAdminEditForm;
 use App\Form\TimesheetMultiUserEditForm;
 use App\Repository\Query\TimesheetQuery;
+use App\Repository\Query\TimesheetQueryHint;
 use App\Utils\PageSetup;
 use Doctrine\Common\Collections\ArrayCollection;
 use Symfony\Component\Form\FormInterface;
@@ -180,6 +181,7 @@ final class TimesheetTeamController extends TimesheetAbstractController
     protected function prepareQuery(TimesheetQuery $query): void
     {
         $query->setCurrentUser($this->getUser());
+        $query->addQueryHint(TimesheetQueryHint::USER_PREFERENCES); // e.g. for latest approval
     }
 
     protected function getCreateForm(Timesheet $entry): FormInterface

src/Export/TimesheetExportRepository.php

@@ -11,11 +11,12 @@ namespace App\Export;
 
 use App\Entity\Timesheet;
 use App\Repository\Query\ExportQuery;
+use App\Repository\Query\TimesheetQueryHint;
 use App\Repository\TimesheetRepository;
 
 final class TimesheetExportRepository implements ExportRepositoryInterface
 {
-    public function __construct(private TimesheetRepository $repository)
+    public function __construct(private readonly TimesheetRepository $repository)
     {
     }
 
@@ -41,7 +42,12 @@ final class TimesheetExportRepository implements ExportRepositoryInterface
 
     public function getExportItemsForQuery(ExportQuery $query): iterable
     {
-        return $this->repository->getTimesheetsForQuery($query, true);
+        $query->addQueryHint(TimesheetQueryHint::CUSTOMER_META_FIELDS);
+        $query->addQueryHint(TimesheetQueryHint::PROJECT_META_FIELDS);
+        $query->addQueryHint(TimesheetQueryHint::ACTIVITY_META_FIELDS);
+        $query->addQueryHint(TimesheetQueryHint::USER_PREFERENCES);
+
+        return $this->repository->getTimesheetResult($query)->getResults();
     }
 
     public function getType(): string

src/Repository/Loader/TimesheetLoader.php

@@ -13,6 +13,9 @@ use App\Entity\Activity;
 use App\Entity\Customer;
 use App\Entity\Project;
 use App\Entity\Timesheet;
+use App\Entity\User;
+use App\Repository\Query\TimesheetQuery;
+use App\Repository\Query\TimesheetQueryHint;
 use Doctrine\ORM\EntityManagerInterface;
 
 /**
@@ -23,7 +26,7 @@ final class TimesheetLoader implements LoaderInterface
 {
     public function __construct(
         private readonly EntityManagerInterface $entityManager,
-        private readonly bool $fullyHydrated = false
+        private readonly ?TimesheetQuery $query = null
     )
     {
     }
@@ -50,7 +53,7 @@ final class TimesheetLoader implements LoaderInterface
             return $timesheet->getProject()?->getId();
         }, $results)), function ($value) { return $value !== null; });
 
-        if ($this->fullyHydrated) {
+        if ($this->query !== null && $this->query->hasQueryHint(TimesheetQueryHint::PROJECT_META_FIELDS)) {
             $qb = $em->createQueryBuilder();
             $qb->select('PARTIAL p.{id}', 'meta')
                 ->from(Project::class, 'p')
@@ -60,43 +63,67 @@ final class TimesheetLoader implements LoaderInterface
                 ->execute();
         }
 
-        $qb = $em->createQueryBuilder();
+        if (\count($projectIds) > 0) {
-        /** @var array<Project> $projects */
-        $projects = $qb->select('PARTIAL p.{id}', 'customer')
-            ->from(Project::class, 'p')
-            ->leftJoin('p.customer', 'customer')
-            ->andWhere($qb->expr()->in('p.id', $projectIds))
-            ->getQuery()
-            ->execute();
-
-        if ($this->fullyHydrated) {
-            $customerIds = array_filter(array_unique(array_map(function (Project $project) {
-                return $project->getCustomer()?->getId();
-            }, $projects)), function ($value) { return $value !== null; });
-
             $qb = $em->createQueryBuilder();
-            $qb->select('PARTIAL c.{id}', 'meta')
+            /** @var array<Project> $projects */
-                ->from(Customer::class, 'c')
+            $projects = $qb->select('PARTIAL p.{id}', 'customer')
-                ->leftJoin('c.meta', 'meta')
+                ->from(Project::class, 'p')
-                ->andWhere($qb->expr()->in('c.id', $customerIds))
+                ->leftJoin('p.customer', 'customer')
+                ->andWhere($qb->expr()->in('p.id', $projectIds))
                 ->getQuery()
                 ->execute();
+
+            if ($this->query !== null && $this->query->hasQueryHint(TimesheetQueryHint::CUSTOMER_META_FIELDS)) {
+                $customerIds = array_filter(array_unique(array_map(function (Project $project) {
+                    return $project->getCustomer()?->getId();
+                }, $projects)), function ($value) { return $value !== null; });
+
+                if (\count($customerIds) > 0) {
+                    $qb = $em->createQueryBuilder();
+                    $qb->select('PARTIAL c.{id}', 'meta')
+                        ->from(Customer::class, 'c')
+                        ->leftJoin('c.meta', 'meta')
+                        ->andWhere($qb->expr()->in('c.id', $customerIds))
+                        ->getQuery()
+                        ->execute();
+                }
+            }
         }
 
-        if ($this->fullyHydrated) {
+        if ($this->query !== null && $this->query->hasQueryHint(TimesheetQueryHint::ACTIVITY_META_FIELDS)) {
             $activityIds = array_filter(array_map(function (Timesheet $timesheet) {
                 return $timesheet->getActivity()?->getId();
             }, $results), function ($id): bool {
                 return $id !== null;
             });
 
-            $qb = $em->createQueryBuilder();
+            if (\count($activityIds) > 0) {
-            $qb->select('PARTIAL a.{id}', 'meta')
+                $qb = $em->createQueryBuilder();
-                ->from(Activity::class, 'a')
+                $qb->select('PARTIAL a.{id}', 'meta')
-                ->leftJoin('a.meta', 'meta')
+                    ->from(Activity::class, 'a')
-                ->andWhere($qb->expr()->in('a.id', $activityIds))
+                    ->leftJoin('a.meta', 'meta')
-                ->getQuery()
+                    ->andWhere($qb->expr()->in('a.id', $activityIds))
-                ->execute();
+                    ->getQuery()
+                    ->execute();
+            }
+        }
+
+        if ($this->query !== null && $this->query->hasQueryHint(TimesheetQueryHint::USER_PREFERENCES)) {
+            $userIds = array_filter(array_map(function (Timesheet $timesheet) {
+                return $timesheet->getUser()?->getId();
+            }, $results), function ($id): bool {
+                return $id !== null;
+            });
+
+            if (\count($userIds) > 0) {
+                $qb = $em->createQueryBuilder();
+                $qb->select('PARTIAL u.{id}', 'preferences')
+                    ->from(User::class, 'u')
+                    ->leftJoin('u.preferences', 'preferences')
+                    ->andWhere($qb->expr()->in('u.id', $userIds))
+                    ->getQuery()
+                    ->execute();
+            }
         }
 
         $qb = $em->createQueryBuilder();

src/Repository/Query/TimesheetQuery.php

@@ -42,6 +42,10 @@ class TimesheetQuery extends ActivityQuery implements BillableInterface, DateRan
      * @var array<User>
      */
     private array $users = [];
+    /**
+     * @var array<TimesheetQueryHint>
+     */
+    private array $queryHints = [];
 
     public function __construct(bool $resetTimes = true)
     {
@@ -59,6 +63,16 @@ class TimesheetQuery extends ActivityQuery implements BillableInterface, DateRan
         ]);
     }
 
+    public function addQueryHint(TimesheetQueryHint $hint): void
+    {
+        $this->queryHints[] = $hint;
+    }
+
+    public function hasQueryHint(TimesheetQueryHint $hint): bool
+    {
+        return \in_array($hint, $this->queryHints, true);
+    }
+
     protected function copyFrom(BaseQuery $query): void
     {
         parent::copyFrom($query);

src/Repository/Query/TimesheetQueryHint.php

@@ -0,0 +1,18 @@
+<?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\Query;
+
+enum TimesheetQueryHint
+{
+    case USER_PREFERENCES;
+    case CUSTOMER_META_FIELDS;
+    case PROJECT_META_FIELDS;
+    case ACTIVITY_META_FIELDS;
+}

src/Repository/Result/TimesheetResult.php

@@ -84,7 +84,7 @@ final class TimesheetResult
             /** @var array<Timesheet> $results */
             $results = $this->query->getResult();
 
-            $loader = new TimesheetLoader($this->entityManager, true);
+            $loader = new TimesheetLoader($this->entityManager, $this->timesheetQuery);
             $loader->loadResults($results);
 
             $this->resultCache = $results;
@@ -95,7 +95,7 @@ final class TimesheetResult
 
     public function getPagerfanta(): Pagination
     {
-        $loader = new LoaderQueryPaginator(new TimesheetLoader($this->entityManager), $this->query, $this->getStatistic()->getCount());
+        $loader = new LoaderQueryPaginator(new TimesheetLoader($this->entityManager, $this->timesheetQuery), $this->query, $this->getStatistic()->getCount());
 
         $paginator = new Pagination($loader);
         $paginator->setMaxPerPage($this->timesheetQuery->getPageSize());

src/Repository/TimesheetInvoiceItemRepository.php

@@ -13,20 +13,25 @@ use App\Entity\ExportableItem;
 use App\Entity\Timesheet;
 use App\Invoice\InvoiceItemRepositoryInterface;
 use App\Repository\Query\InvoiceQuery;
+use App\Repository\Query\TimesheetQueryHint;
 
 final class TimesheetInvoiceItemRepository implements InvoiceItemRepositoryInterface
 {
-    public function __construct(private TimesheetRepository $repository)
+    public function __construct(private readonly TimesheetRepository $repository)
     {
     }
 
     /**
-     * @param InvoiceQuery $query
      * @return ExportableItem[]
      */
     public function getInvoiceItemsForQuery(InvoiceQuery $query): iterable
     {
-        return $this->repository->getTimesheetsForQuery($query, true);
+        $query->addQueryHint(TimesheetQueryHint::CUSTOMER_META_FIELDS);
+        $query->addQueryHint(TimesheetQueryHint::PROJECT_META_FIELDS);
+        $query->addQueryHint(TimesheetQueryHint::ACTIVITY_META_FIELDS);
+        $query->addQueryHint(TimesheetQueryHint::USER_PREFERENCES);
+
+        return $this->repository->getTimesheetResult($query)->getResults();
     }
 
     /**

src/Repository/TimesheetRepository.php

@@ -24,6 +24,7 @@ use App\Repository\Loader\TimesheetLoader;
 use App\Repository\Paginator\LoaderQueryPaginator;
 use App\Repository\Paginator\PaginatorInterface;
 use App\Repository\Query\TimesheetQuery;
+use App\Repository\Query\TimesheetQueryHint;
 use App\Repository\Result\TimesheetResult;
 use App\Utils\Pagination;
 use DateInterval;
@@ -472,12 +473,11 @@ class TimesheetRepository extends EntityRepository
         $counter = $this->countTimesheetsForQuery($timesheetQuery);
         $query = $this->createTimesheetQuery($timesheetQuery);
 
-        return new LoaderQueryPaginator(new TimesheetLoader($this->getEntityManager()), $query, $counter);
+        return new LoaderQueryPaginator(new TimesheetLoader($this->getEntityManager(), $timesheetQuery), $query, $counter);
     }
 
     /**
-     * When switching $fullyHydrated to true, the call gets even more expensive.
+     * TODO @deprecated since 2.25 - use getTimesheetResult() with TimesheetQueryHint instead
-     * You normally don't need this, unless you want to access deeply nested attributes for many entries.
      *
      * @return Timesheet[]
      */
@@ -485,7 +485,13 @@ class TimesheetRepository extends EntityRepository
     {
         $qb = $this->getQueryBuilderForQuery($query);
 
-        return $this->getHydratedResultsByQuery($qb, $fullyHydrated);
+        if ($fullyHydrated) {
+            $query->addQueryHint(TimesheetQueryHint::CUSTOMER_META_FIELDS);
+            $query->addQueryHint(TimesheetQueryHint::PROJECT_META_FIELDS);
+            $query->addQueryHint(TimesheetQueryHint::ACTIVITY_META_FIELDS);
+        }
+
+        return $this->getHydratedResultsByQuery($qb, $query);
     }
 
     public function getTimesheetResult(TimesheetQuery $query): TimesheetResult
@@ -501,16 +507,16 @@ class TimesheetRepository extends EntityRepository
     /**
      * @return Timesheet[]
      */
-    private function getHydratedResultsByQuery(QueryBuilder $qb, bool $fullyHydrated = false): array
+    private function getHydratedResultsByQuery(QueryBuilder $qb, ?TimesheetQuery $timesheetQuery = null): array
     {
         /** @var Query<Timesheet> $query */
         $query = $qb->getQuery();
-        $query = $this->prepareTimesheetQuery($query);
+        $query = $this->prepareTimesheetQuery($query, $timesheetQuery);
 
         /** @var array<Timesheet> $timesheets */
         $timesheets = $query->getResult();
 
-        $loader = new TimesheetLoader($qb->getEntityManager(), $fullyHydrated);
+        $loader = new TimesheetLoader($qb->getEntityManager(), $timesheetQuery);
         $loader->loadResults($timesheets);
 
         return $timesheets;
@@ -914,7 +920,7 @@ class TimesheetRepository extends EntityRepository
     private function createTimesheetQuery(TimesheetQuery $timesheetQuery): Query
     {
         $query = $this->getQueryBuilderForQuery($timesheetQuery)->getQuery();
-        $query = $this->prepareTimesheetQuery($query);
+        $query = $this->prepareTimesheetQuery($query, $timesheetQuery);
 
         return $query;
     }
@@ -923,7 +929,7 @@ class TimesheetRepository extends EntityRepository
      * @param Query<Timesheet> $query
      * @return Query<Timesheet>
      */
-    public function prepareTimesheetQuery(Query $query): Query
+    public function prepareTimesheetQuery(Query $query, ?TimesheetQuery $timesheetQuery = null): Query
     {
         $this->getEntityManager()->getConfiguration()->setEagerFetchBatchSize(300);
 
@@ -932,15 +938,6 @@ class TimesheetRepository extends EntityRepository
         $query->setFetchMode(Timesheet::class, 'project', ClassMetadata::FETCH_EAGER);
         $query->setFetchMode(Timesheet::class, 'user', ClassMetadata::FETCH_EAGER);
 
-        // not yet supported by Doctrine
-        // $query->setFetchMode(Activity::class, 'meta', ClassMetadata::FETCH_EAGER);
-        // $query->setFetchMode(Project::class, 'customer', ClassMetadata::FETCH_EAGER);
-        // $query->setFetchMode(Project::class, 'meta', ClassMetadata::FETCH_EAGER);
-        // $query->setFetchMode(Customer::class, 'meta', ClassMetadata::FETCH_EAGER);
-
-        // ManyToMany not supported by Doctrine yet
-        // $query->setFetchMode(Timesheet::class, 'tags', ClassMetadata::FETCH_EAGER);
-
         return $query;
     }
 }

src/Repository/WorkingTimeRepository.php

@@ -66,10 +66,10 @@ class WorkingTimeRepository extends EntityRepository
         return $qb->getQuery()->getResult();
     }
 
-    public function getLatestApproval(User $user): ?WorkingTime
+    public function getLatestApprovalDate(User $user): ?\DateTimeInterface
     {
         $qb = $this->createQueryBuilder('w');
-        $qb->select('MAX(DATE(w.date))')
+        $qb->select($qb->expr()->max('(DATE(w.date))'))
             ->where($qb->expr()->eq('w.user', ':user'))
             ->setParameter('user', $user->getId())
             ->andWhere($qb->expr()->isNotNull('w.approvedAt'))
@@ -81,14 +81,6 @@ class WorkingTimeRepository extends EntityRepository
             return null;
         }
 
-        $qb = $this->createQueryBuilder('w');
+        return new \DateTimeImmutable($date . ' 00:00:00', new \DateTimeZone($user->getTimezone()));
-        $qb->select('w')
-            ->where($qb->expr()->eq('w.user', ':user'))
-            ->setParameter('user', $user->getId())
-            ->andWhere($qb->expr()->eq('DATE(w.date)', 'DATE(:date)'))
-            ->setParameter('date', $date)
-        ;
-
-        return $qb->getQuery()->getOneOrNullResult(); // @phpstan-ignore-line
     }
 }

src/WorkingTime/WorkingTimeService.php

@@ -15,6 +15,7 @@ use App\Event\WorkingTimeApproveMonthEvent;
 use App\Event\WorkingTimeYearEvent;
 use App\Event\WorkingTimeYearSummaryEvent;
 use App\Repository\TimesheetRepository;
+use App\Repository\UserRepository;
 use App\Repository\WorkingTimeRepository;
 use App\Timesheet\DateTimeFactory;
 use App\WorkingTime\Mode\WorkingTimeMode;
@@ -29,14 +30,15 @@ use Psr\EventDispatcher\EventDispatcherInterface;
  */
 final class WorkingTimeService
 {
-    /** @var array<string, WorkingTime|null> */
+    private const LATEST_APPROVAL_PREF = '_latest_approval';
-    private array $latestApprovals = [];
+    private const LATEST_APPROVAL_FORMAT = 'Y-m-d H:i:s';
 
     public function __construct(
         private readonly TimesheetRepository $timesheetRepository,
         private readonly WorkingTimeRepository $workingTimeRepository,
         private readonly EventDispatcherInterface $eventDispatcher,
-        private readonly WorkingTimeModeFactory $contractModeService
+        private readonly WorkingTimeModeFactory $contractModeService,
+        private readonly UserRepository $userRepository,
     )
     {
     }
@@ -56,30 +58,42 @@ final class WorkingTimeService
         return $yearPerUserSummary;
     }
 
-    public function getLatestApproval(User $user): ?WorkingTime
+    public function getLatestApprovalDate(User $user): ?\DateTimeInterface
     {
         if ($user->getId() === null) {
             return null;
         }
 
-        $key = 'u_' . $user->getId();
+        $date = $user->getPreferenceValue(self::LATEST_APPROVAL_PREF, false);
 
-        if (!\array_key_exists($key, $this->latestApprovals)) {
+        // false means = there is no setting existing: let's calculate it
-            $this->latestApprovals[$key] = $this->workingTimeRepository->getLatestApproval($user);
+        // null means = there is no approval existing yet
+        if ($date === false) {
+            $date = $this->workingTimeRepository->getLatestApprovalDate($user);
+
+            // let's store the approval always: we can later detect if an approval exists
+            // or not based on the existence of the preference, which saves DB queries
+            $value = ($date !== null) ? $date->format(self::LATEST_APPROVAL_FORMAT) : null;
+            $user->setPreferenceValue(self::LATEST_APPROVAL_PREF, $value);
+            $this->userRepository->saveUser($user);
+
+            return $date;
         }
 
-        return $this->latestApprovals[$key];
+        if (\is_string($date)) {
+            return new \DateTimeImmutable($date, new \DateTimeZone($user->getTimezone()));
+        }
+
+        return null;
     }
 
     public function isApproved(User $user, \DateTimeInterface $dateTime): bool
     {
-        $latestApproval = $this->getLatestApproval($user);
+        $latestApprovalDate = $this->getLatestApprovalDate($user);
-        if ($latestApproval === null) {
+        if ($latestApprovalDate === null) {
             return false;
         }
 
-        $latestApprovalDate = $latestApproval->getDate();
-
         $begin = \DateTimeImmutable::createFromInterface($dateTime);
         $begin = $begin->setTime(0, 0, 0);
 
@@ -170,10 +184,8 @@ final class WorkingTimeService
 
         $this->workingTimeRepository->persistScheduledWorkingTimes();
 
-        $key = 'u_' . $user->getId();
+        $user->setPreferenceValue(self::LATEST_APPROVAL_PREF, $this->workingTimeRepository->getLatestApprovalDate($user)?->format(self::LATEST_APPROVAL_FORMAT));
-        if (\array_key_exists($key, $this->latestApprovals)) {
+        $this->userRepository->saveUser($user);
-            unset($this->latestApprovals[$key]);
-        }
 
         $this->eventDispatcher->dispatch(new WorkingTimeApproveMonthEvent($user, $month, $approvalDate, $approvedBy));
     }

tests/Repository/TimesheetRepositoryTest.php

@@ -17,6 +17,7 @@ use App\Entity\User;
 use App\Repository\ActivityRepository;
 use App\Repository\ProjectRepository;
 use App\Repository\Query\TimesheetQuery;
+use App\Repository\Query\TimesheetQueryHint;
 use App\Repository\TimesheetRepository;
 use App\Utils\Pagination;
 
@@ -36,8 +37,15 @@ class TimesheetRepositoryTest extends AbstractRepositoryTest
 
         $result = $repository->getPagerfantaForQuery($query);
         $this->assertInstanceOf(Pagination::class, $result);
+        self::assertFalse($query->hasQueryHint(TimesheetQueryHint::CUSTOMER_META_FIELDS));
+        self::assertFalse($query->hasQueryHint(TimesheetQueryHint::PROJECT_META_FIELDS));
+        self::assertFalse($query->hasQueryHint(TimesheetQueryHint::ACTIVITY_META_FIELDS));
 
-        $result = $repository->getTimesheetsForQuery($query);
+        $result = $repository->getTimesheetsForQuery($query, true);
+
+        self::assertTrue($query->hasQueryHint(TimesheetQueryHint::CUSTOMER_META_FIELDS));
+        self::assertTrue($query->hasQueryHint(TimesheetQueryHint::PROJECT_META_FIELDS));
+        self::assertTrue($query->hasQueryHint(TimesheetQueryHint::ACTIVITY_META_FIELDS));
         $this->assertIsArray($result);
     }