Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Improve performance of backend module with many log entries #198

Merged
merged 5 commits into from Mar 26, 2024
Merged

Improve performance of backend module with many log entries #198

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
8f76b59
[TASK] Improve performance of backend module with many log entries [T…
bmgrieger Mar 20, 2024
5c5b5f5
[TASK] Run PHP code style fixer [TER-187] [TER-188]
bmgrieger Mar 20, 2024
6b933c1
[TASK] Implement code improvements [TER-187] [TER-188]
bmgrieger Mar 22, 2024
af70d1f
Merge branch 'master' into feature/TER-187-v12
bmgrieger Mar 22, 2024
31feac7
[TASK] Prepare release of version 6.0.2 [TER-187] [TER-188]
bmgrieger Mar 22, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
27 changes: 16 additions & 11 deletions Classes/Controller/LogController.php
View file
Open in desktop
Expand Up @@ -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;

Expand Down Expand Up @@ -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');
Expand Down
202 changes: 100 additions & 102 deletions Classes/Domain/Repository/LogRepository.php
View file
Open in desktop
Expand Up @@ -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);
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would put $queryBuilder->...->fetchOne() ?? 0 into parentheses, so that the cast goes against the whole construct (not really necessary here, but looks better to me).


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);
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same as above


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);
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same as above


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));
}
}

Expand Down Expand Up @@ -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();
}
}
52 changes: 20 additions & 32 deletions Classes/Domain/Transfer/Statistic.php
View file
Open in desktop
Expand Up @@ -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
Expand Down