Improve performance of backend module with many log entries (#198)

* [TASK] Improve performance of backend module with many log entries [TER-187] [TER-188] * [TASK] Run PHP code style fixer [TER-187] [TER-188] * [TASK] Implement code improvements [TER-187] [TER-188] * [TASK] Prepare release of version 6.0.2 [TER-187] [TER-188]
Leuchtfeuer · Mar 26, 2024 · aff5928 · aff5928
1 parent d58136d
commit aff5928
Show file tree

Hide file tree

Showing 8 changed files with 210 additions and 168 deletions.
diff --git a/Classes/Controller/LogController.php b/Classes/Controller/LogController.php
@@ -25,8 +25,6 @@
 use TYPO3\CMS\Core\Configuration\ExtensionConfiguration;
 use TYPO3\CMS\Core\Database\ConnectionPool;
 use TYPO3\CMS\Core\Page\PageRenderer;
-use TYPO3\CMS\Core\Pagination\ArrayPaginator;
-use TYPO3\CMS\Core\Pagination\SimplePagination;
 use TYPO3\CMS\Core\Utility\GeneralUtility;
 use TYPO3\CMS\Extbase\Mvc\Controller\ActionController;
 
@@ -65,29 +63,36 @@ public function listAction(?Filter $filter = null): ResponseInterface
 
         $pageId = (int)(array_key_exists('id', $this->request->getQueryParams()) ? $this->request->getQueryParams()['id'] : 0);
         $filter->setPageId($pageId);
-        $logEntries = $this->logRepository->findByFilter($filter);
 
         $this->persistFilterInBeUserData($filter);
         $this->resetFilterOnMemoryExhaustionError();
 
         $itemsPerPage = 20;
-        $currentPage = (int)array_key_exists('currentPage', $this->request->getQueryParams()) && $this->request->getQueryParams()['currentPage'] > 0 ? $this->request->getQueryParams()['currentPage'] : 1;
+        $currentPage = (int)(array_key_exists('currentPage', $this->request->getQueryParams()) && $this->request->getQueryParams()['currentPage'] > 0 ? $this->request->getQueryParams()['currentPage'] : 1);
+        $logEntries = $this->logRepository->findByFilter($filter, $currentPage, $itemsPerPage);
 
-        $paginator = new ArrayPaginator($logEntries->toArray(), $currentPage, $itemsPerPage);
-        $pagination = new SimplePagination($paginator);
+        $totalResultsCount = $this->logRepository->countByFilter($filter);
+        $totalPages = (int)(ceil($totalResultsCount / $itemsPerPage));
+
+        $statistic = new Statistic();
+        $statistic->calc($filter, $this->logRepository);
 
         $moduleTemplate = $this->moduleTemplateFactory->create($this->request);
         $moduleTemplate->assignMultiple([
             'loggingEnabled' => $extensionConfigurationLogging,
-            'logs' => $paginator->getPaginatedItems(),
+            'logs' => $logEntries,
             'page' => BackendUtility::getRecord('pages', $pageId),
             'users' => $this->getUsers(),
             'fileTypes' => $this->getFileTypes(),
             'filter' => $filter,
-            'statistic' => new Statistic($logEntries),
-            'paginator' => $paginator,
-            'pagination' => $pagination,
-            'totalResultCount' => count($logEntries),
+            'statistic' => $statistic,
+            'pagination' => [
+                'totalPages' => $totalPages,
+                'currentPage' => $currentPage,
+                'previousPage' => ($currentPage - 1) > 0 ? $currentPage - 1 : 0,
+                'nextPage' => $totalPages > $currentPage ? $currentPage + 1 : 0,
+            ],
+            'totalResultCount' => $totalResultsCount,
             'isRoot' => $pageId == 0,
         ]);
         return $moduleTemplate->renderResponse('List');

diff --git a/Classes/Domain/Repository/LogRepository.php b/Classes/Domain/Repository/LogRepository.php
@@ -17,154 +17,149 @@
 use Leuchtfeuer\SecureDownloads\Domain\Model\Log;
 use Leuchtfeuer\SecureDownloads\Domain\Transfer\Filter;
 use Leuchtfeuer\SecureDownloads\Domain\Transfer\Token\AbstractToken;
+use TYPO3\CMS\Core\Database\Connection;
 use TYPO3\CMS\Core\Database\ConnectionPool;
+use TYPO3\CMS\Core\Database\Query\QueryBuilder;
 use TYPO3\CMS\Core\Resource\Exception\ResourceDoesNotExistException;
 use TYPO3\CMS\Core\Resource\ResourceFactory;
 use TYPO3\CMS\Core\Utility\GeneralUtility;
 use TYPO3\CMS\Extbase\Persistence\Exception\InvalidQueryException;
-use TYPO3\CMS\Extbase\Persistence\QueryInterface;
-use TYPO3\CMS\Extbase\Persistence\QueryResultInterface;
+use TYPO3\CMS\Extbase\Persistence\Generic\Mapper\DataMapper;
 use TYPO3\CMS\Extbase\Persistence\Repository;
 
 class LogRepository extends Repository
 {
-    protected $defaultOrderings = [
-        'tstamp' => QueryInterface::ORDER_DESCENDING,
-    ];
+    const TABLENAME = 'tx_securedownloads_domain_model_log';
 
-    /**
-     * Initializes the query and applies default options.
-     *
-     * @return QueryInterface The generated query object.
-     */
-    public function createQuery(): QueryInterface
+    public function __construct(
+        private readonly ConnectionPool $connectionPool,
+        private readonly DataMapper $dataMapper
+    ) {
+    }
+
+    public function createQueryBuilder(): QueryBuilder
     {
-        $query = parent::createQuery();
-        $querySettings = $query->getQuerySettings();
-        $querySettings->setRespectStoragePage(false);
-        $querySettings->setRespectSysLanguage(false);
-        $query->setQuerySettings($querySettings);
+        $queryBuilder = $this->connectionPool->getQueryBuilderForTable(self::TABLENAME);
+        $queryBuilder->getRestrictions()->removeAll();
+        $queryBuilder
+            ->from(self::TABLENAME)
+            ->orderBy('tstamp', 'DESC');
 
-        return $query;
+        return $queryBuilder;
     }
 
-    /**
-     * Finds log data and applies filter.
-     *
-     * @param Filter|null $filter The filter object.
-     *
-     * @return QueryResultInterface The query result.
-     */
-    public function findByFilter(?Filter $filter): QueryResultInterface
+    public function findByFilter(?Filter $filter, int $currentPage = 1, int $itemsPerPage = 20): array
     {
-        $query = $this->createQuery();
+        $queryBuilder = $this->createQueryBuilder();
 
-        if ($filter instanceof Filter) {
-            try {
-                $this->applyFilter($query, $filter);
-            } catch (InvalidQueryException $exception) {
-                // Do nothing for now.
-            }
-        }
+        $this->applyFilter($queryBuilder, $filter);
 
-        return $query->execute();
+        $result = $queryBuilder
+            ->select('*')
+            ->setMaxResults($itemsPerPage)
+            ->setFirstResult($itemsPerPage * ($currentPage - 1))
+            ->executeQuery()
+            ->fetchAllAssociative() ?? [];
+        return $this->dataMapper->map(Log::class, $result);
     }
 
-    /**
-     * Applies the filter to a query object.
-     *
-     * @param QueryInterface $query  The query object
-     * @param Filter         $filter The filter object
-     * @throws InvalidQueryException
-     */
-    protected function applyFilter(QueryInterface &$query, Filter $filter): void
+    public function countByFilter(?Filter $filter): int
+    {
+        $queryBuilder = $this->createQueryBuilder();
+
+        $this->applyFilter($queryBuilder, $filter);
+
+        return (int)($queryBuilder
+            ->count('uid')
+            ->executeQuery()
+            ->fetchOne() ?? 0);
+    }
+
+    public function getFirstTimestampByFilter(?Filter $filter, bool $reverse = false): int
+    {
+        $queryBuilder = $this->createQueryBuilder();
+
+        $this->applyFilter($queryBuilder, $filter);
+
+        return (int)($queryBuilder
+            ->select('tstamp')
+            ->orderBy('tstamp', $reverse ? 'DESC' : 'ASC')
+            ->executeQuery()
+            ->fetchOne() ?? 0);
+    }
+
+    public function getTrafficSumByFilter(?Filter $filter): float
+    {
+        $queryBuilder = $this->createQueryBuilder();
+
+        $this->applyFilter($queryBuilder, $filter);
+
+        return (float)($queryBuilder
+            ->selectLiteral('SUM(file_size) AS sum')
+            ->executeQuery()
+            ->fetchOne() ?? 0.0);
+    }
+
+    protected function applyFilter(QueryBuilder &$queryBuilder, Filter $filter): void
     {
         $constraints = [];
 
-        // FileType
-        $this->applyFileTypePropertyToFilter($filter->getFileType(), $query, $constraints);
+        if ($filter instanceof Filter) {
+            try {
+                // FileType
+                $this->applyFileTypePropertyToFilter($filter->getFileType(), $queryBuilder, $constraints);
 
-        // User Type
-        $this->applyUserTypePropertyToFilter($filter, $query, $constraints);
+                // User Type
+                $this->applyUserTypePropertyToFilter($filter, $queryBuilder, $constraints);
 
-        // Period
-        $this->applyPeriodPropertyToFilter($filter, $query, $constraints);
+                // Period
+                $this->applyPeriodPropertyToFilter($filter, $queryBuilder, $constraints);
 
-        // User and Page
-        $this->applyEqualPropertyToFilter((int)$filter->getFeUserId(), 'user', $query, $constraints);
-        $this->applyEqualPropertyToFilter((int)$filter->getPageId(), 'page', $query, $constraints);
+                // User and Page
+                $this->applyEqualPropertyToFilter((int)$filter->getFeUserId(), 'user', $queryBuilder, $constraints);
+                $this->applyEqualPropertyToFilter((int)$filter->getPageId(), 'page', $queryBuilder, $constraints);
 
-        if (count($constraints) > 0) {
-            $query->matching($query->logicalAnd(...$constraints));
+                if (count($constraints) > 0) {
+                    $queryBuilder->where(...$constraints);
+                }
+            } catch (InvalidQueryException $exception) {
+                // Do nothing for now.
+            }
         }
     }
 
-    /**
-     * Applies the file type property of the filter to the query object.
-     *
-     * @param mixed          $fileType    Identifier of the file type
-     * @param QueryInterface $query       The query object
-     * @param array          $constraints Array containing all previously applied constraints
-     */
-    protected function applyFileTypePropertyToFilter($fileType, QueryInterface $query, array &$constraints): void
+    protected function applyFileTypePropertyToFilter(string $fileType, QueryBuilder $queryBuilder, array &$constraints): void
     {
         if ($fileType !== '' && $fileType !== '0') {
-            $constraints[] = $query->equals('mediaType', $fileType);
+            $constraints[] = $queryBuilder->expr()->eq('media_type', $queryBuilder->createNamedParameter($fileType));
         }
     }
 
-    /**
-     * Applies the user type property of the filter to the query object.
-     *
-     * @param Filter         $filter      The filter object
-     * @param QueryInterface $query       The query object
-     * @param array          $constraints Array containing all previously applied constraints
-     */
-    protected function applyUserTypePropertyToFilter(Filter $filter, QueryInterface $query, array &$constraints): void
+    protected function applyUserTypePropertyToFilter(Filter $filter, QueryBuilder $queryBuilder, array &$constraints): void
     {
-        if ($filter->getUserType() != 0) {
-            $userQuery = $query->equals('user', null);
-
-            if ($filter->getUserType() === Filter::USER_TYPE_LOGGED_ON) {
-                $constraints[] = $query->logicalNot($userQuery);
-            }
-            if ($filter->getUserType() === Filter::USER_TYPE_LOGGED_OFF) {
-                $constraints[] = $userQuery;
-            }
+        if ($filter->getUserType() === Filter::USER_TYPE_LOGGED_ON) {
+            $constraints[] = $queryBuilder->expr()->gt('user', $queryBuilder->createNamedParameter(0, Connection::PARAM_INT));
+        }
+        if ($filter->getUserType() === Filter::USER_TYPE_LOGGED_OFF) {
+            $constraints[] = $queryBuilder->expr()->eq('user', $queryBuilder->createNamedParameter(0, Connection::PARAM_INT));
         }
     }
 
-    /**
-     * Applies the period properties of the filter to the query object.
-     *
-     * @param Filter         $filter      The filter object
-     * @param QueryInterface $query       The query object
-     * @param array          $constraints Array containing all previously applied constraints
-     * @throws InvalidQueryException
-     */
-    protected function applyPeriodPropertyToFilter(Filter $filter, QueryInterface $query, array &$constraints): void
+    protected function applyPeriodPropertyToFilter(Filter $filter, QueryBuilder $queryBuilder, array &$constraints): void
     {
         if ((int)$filter->getFrom() !== 0) {
-            $constraints[] = $query->greaterThanOrEqual('tstamp', $filter->getFrom());
+            $constraints[] = $queryBuilder->expr()->gte('tstamp', $queryBuilder->createNamedParameter($filter->getFrom(), Connection::PARAM_INT));
         }
 
         if ((int)$filter->getTill() !== 0) {
-            $constraints[] = $query->lessThanOrEqual('tstamp', $filter->getTill());
+            $constraints[] = $queryBuilder->expr()->lte('tstamp', $queryBuilder->createNamedParameter($filter->getTill(), Connection::PARAM_INT));
         }
     }
 
-    /**
-     * Applies given property of the filter to the query object.
-     *
-     * @param int            $property     The value of the property
-     * @param string         $propertyName The property name
-     * @param QueryInterface $query        The query object
-     * @param array          $constraints  Array containing all previously applied constraints
-     */
-    protected function applyEqualPropertyToFilter(int $property, string $propertyName, QueryInterface $query, array &$constraints): void
+    protected function applyEqualPropertyToFilter(int $property, string $propertyName, QueryBuilder $queryBuilder, array &$constraints): void
     {
         if ($property !== 0) {
-            $constraints[] = $query->equals($propertyName, $property);
+            $constraints[] = $queryBuilder->expr()->eq($propertyName, $queryBuilder->createNamedParameter($property, Connection::PARAM_INT));
         }
     }
 
@@ -196,7 +191,10 @@ public function logDownload(AbstractToken $token, int $fileSize, string $mimeTyp
             $log->setFileId((string)$fileObject->getUid());
         }
 
-        $queryBuilder = GeneralUtility::makeInstance(ConnectionPool::class)->getQueryBuilderForTable('tx_securedownloads_domain_model_log');
-        $queryBuilder->insert('tx_securedownloads_domain_model_log')->values($log->toArray())->executeStatement();
+        $queryBuilder = $this->createQueryBuilder();
+        $queryBuilder
+            ->insert(self::TABLENAME)
+            ->values($log->toArray())
+            ->executeStatement();
     }
 }
diff --git a/Classes/Domain/Transfer/Statistic.php b/Classes/Domain/Transfer/Statistic.php
@@ -14,45 +14,33 @@
  *
  ***/
 
-use Leuchtfeuer\SecureDownloads\Domain\Model\Log;
-use TYPO3\CMS\Extbase\Persistence\QueryResultInterface;
+use Leuchtfeuer\SecureDownloads\Domain\Repository\LogRepository;
 
 class Statistic
 {
-    /**
-     * @var float
-     */
-    protected $traffic = 0.00;
-
-    /**
-     * @var \DateTime
-     */
-    protected $from;
-
-    /**
-     * @var \DateTime
-     */
-    protected $till;
-
-    public function __construct(QueryResultInterface $logEntries)
+    public function __construct(
+        protected \DateTime $from = new \DateTime(),
+        protected \DateTime $till = new \DateTime(),
+        protected float $traffic = 0.00
+    )
     {
-        $this->from = new \DateTime();
-        $this->till = new \DateTime();
-        $count = $logEntries->count();
+    }
 
-        if ($count > 0) {
-            $this->till->setTimestamp($logEntries->getFirst()->getTstamp());
-            $i = 1;
+    public function calc(Filter $filter, LogRepository $logRepository): void
+    {
+        if ($filter->getFrom() !== null) {
+            $this->from->setTimestamp($filter->getFrom());
+        } else {
+            $this->from->setTimestamp($logRepository->getFirstTimestampByFilter($filter));
+        }
 
-            /** @var Log $logEntry */
-            foreach ($logEntries as $logEntry) {
-                $this->traffic += $logEntry->getFileSize();
-                if ($i === $count) {
-                    $this->from->setTimestamp($logEntry->getTstamp());
-                }
-                $i++;
-            }
+        if ($filter->getTill() !== null) {
+            $this->till->setTimestamp($filter->getTill());
+        } elseif ($logRepository->getFirstTimestampByFilter($filter, true) > 0) {
+            $this->till->setTimestamp($logRepository->getFirstTimestampByFilter($filter, true));
         }
+
+        $this->traffic = $logRepository->getTrafficSumByFilter($filter);
     }
 
     public function getTraffic(): float