Skip to content

Commit

Permalink
ExApp caching: "exApp_" -> "ex_apps" (#253)
Browse files Browse the repository at this point in the history
Should significantly improve the performance of anything that uses the
getExAppsList function:

1. Admin page
2. ExAppUIL10N middleware
3. And in many other places where there were multiple cyclic calls to
`getExApp`

---------

Signed-off-by: Alexander Piskun <[email protected]>
  • Loading branch information
bigcat88 authored Mar 20, 2024
1 parent bab1078 commit 8c06643
Show file tree
Hide file tree
Showing 3 changed files with 39 additions and 30 deletions.
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -11,3 +11,4 @@ vendor
_build
certs
.php-cs-fixer.cache
dev
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
### Fixed

- Fixed incorrect notifications handling producing a lot of errors in the log. #252
- Replaced single ExApp caching with list caching, should improve performance. #253

## [2.3.1 - 2024-03-18]

Expand Down
67 changes: 37 additions & 30 deletions lib/Service/ExAppService.php
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@
use SimpleXMLElement;

class ExAppService {
public const CACHE_TTL = 60 * 60; // 1 hour
private ICache $cache;

public function __construct(
Expand Down Expand Up @@ -58,18 +57,12 @@ public function __construct(
}

public function getExApp(string $appId): ?ExApp {
try {
$cacheKey = '/exApp_' . $appId;
$cached = $this->cache->get($cacheKey);
if ($cached !== null) {
return $cached instanceof ExApp ? $cached : new ExApp($cached);
foreach ($this->getExApps() as $exApp) {
if ($exApp->getAppid() === $appId) {
return $exApp;
}

$exApp = $this->exAppMapper->findByAppId($appId);
$this->cache->set($cacheKey, $exApp, self::CACHE_TTL);
return $exApp;
} catch (Exception | MultipleObjectsReturnedException | DoesNotExistException) {
}
$this->logger->debug(sprintf('ExApp "%s" not found.', $appId));
return null;
}

Expand All @@ -89,7 +82,7 @@ public function registerExApp(array $appInfo): ?ExApp {
try {
$this->exAppMapper->insert($exApp);
$exApp = $this->exAppMapper->findByAppId($appInfo['id']);
$this->cache->set('/exApp_' . $appInfo['id'], $exApp, self::CACHE_TTL);
$this->cache->remove('/ex_apps');
return $exApp;
} catch (Exception | MultipleObjectsReturnedException | DoesNotExistException $e) {
$this->logger->error(sprintf('Error while registering ExApp %s: %s', $appInfo['id'], $e->getMessage()));
Expand Down Expand Up @@ -119,7 +112,7 @@ public function unregisterExApp(string $appId): bool {
if ($r !== 1) {
$this->logger->error(sprintf('Error while unregistering %s ExApp from the database.', $appId));
}
$this->cache->remove('/exApp_' . $appId);
$this->cache->remove('/ex_apps');
return $r === 1;
}

Expand Down Expand Up @@ -166,23 +159,15 @@ public function getExAppsByDaemonName(string $daemonName): array {
}

public function getExAppsList(string $list = 'enabled'): array {
try {
$exApps = $this->exAppMapper->findAll();

if ($list === 'enabled') {
$exApps = array_values(array_filter($exApps, function (ExApp $exApp) {
return $exApp->getEnabled() === 1;
}));
}

$exApps = array_map(function (ExApp $exApp) {
return $this->formatExAppInfo($exApp);
}, $exApps);
} catch (Exception $e) {
$this->logger->error(sprintf('Error while getting ExApps list. Error: %s', $e->getMessage()), ['exception' => $e]);
$exApps = [];
$exApps = $this->getExApps();
if ($list === 'enabled') {
$exApps = array_values(array_filter($exApps, function (ExApp $exApp) {
return $exApp->getEnabled() === 1;
}));
}
return $exApps;
return array_map(function (ExApp $exApp) {
return $this->formatExAppInfo($exApp);
}, $exApps);
}

public function formatExAppInfo(ExApp $exApp): array {
Expand Down Expand Up @@ -219,7 +204,7 @@ public function updateExAppInfo(ExApp $exApp, array $appInfo): bool {
public function updateExApp(ExApp $exApp, array $fields = ['version', 'name', 'port', 'status', 'enabled', 'last_check_time', 'is_system']): bool {
try {
$this->exAppMapper->updateExApp($exApp, $fields);
$this->cache->set('/exApp_' . $exApp->getAppid(), $exApp, self::CACHE_TTL);
$this->cache->remove('/ex_apps');
return true;
} catch (Exception $e) {
$this->logger->error(sprintf('Failed to update "%s" ExApp info.', $exApp->getAppid()), ['exception' => $e]);
Expand Down Expand Up @@ -342,4 +327,26 @@ public function setStatusError(ExApp $exApp, string $error): void {
$exApp->setStatus($status);
$this->updateExApp($exApp, ['status']);
}

/**
* Get list of registered ExApps
*
* @return ExApp[]
*/
public function getExApps(): array {
try {
$cacheKey = '/ex_apps';
$records = $this->cache->get($cacheKey);
if ($records !== null) {
return array_map(function ($record) {
return $record instanceof ExApp ? $record : new ExApp($record);
}, $records);
}
$records = $this->exAppMapper->findAll();
$this->cache->set($cacheKey, $records);
return $records;
} catch (Exception) {
return [];
}
}
}

0 comments on commit 8c06643

Please sign in to comment.