Skip to content

Commit 46e6fa2

Browse files
oleksandr-ncoleksandr-nc
authored
Merge pull request #265 from nextcloud/perf/lazy-app-config
perf: load the app config values lazily
2 parents 3386998 + f6dd915 commit 46e6fa2

18 files changed

Lines changed: 917 additions & 58 deletions

‎CHANGELOG.md‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,10 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
1010

1111
## [Unreleased]
1212

13+
### Changed
14+
15+
- Stop loading most of the app's settings on every request
16+
1317
## 3.3.0 - 2026-10-02
1418

1519
### Added

‎lib/AppInfo/Application.php‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
use OCP\AppFramework\Bootstrap\IBootContext;
2020
use OCP\AppFramework\Bootstrap\IBootstrap;
2121
use OCP\AppFramework\Bootstrap\IRegistrationContext;
22+
use OCP\AppFramework\Services\IAppConfig;
2223

2324
use OCP\Collaboration\Reference\RenderReferenceEvent;
2425

@@ -37,20 +38,22 @@ class Application extends App implements IBootstrap {
3738

3839
private ContainerInterface $container;
3940
private IConfig $config;
41+
private IAppConfig $appConfig;
4042

4143
public function __construct(array $urlParams = []) {
4244
parent::__construct(self::APP_ID, $urlParams);
4345

4446
$this->container = $this->getContainer();
4547
$this->config = $this->container->get(IConfig::class);
48+
$this->appConfig = $this->container->get(IAppConfig::class);
4649
}
4750

4851
public function register(IRegistrationContext $context): void {
49-
if ($this->config->getAppValue(self::APP_ID, 'dashboard_enabled', '1') === '1') {
52+
if ($this->appConfig->getAppValueString('dashboard_enabled', '1') === '1') {
5053
$context->registerDashboardWidget(GithubWidget::class);
5154
}
5255

53-
if ($this->config->getAppValue(self::APP_ID, 'issue_notifications_enabled', '1') === '1') {
56+
if ($this->appConfig->getAppValueString('issue_notifications_enabled', '1') === '1') {
5457
$context->registerNotifierService(GitHubNotifier::class);
5558
}
5659

‎lib/Controller/ConfigController.php‎

Lines changed: 33 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
use OCP\AppFramework\Http\DataResponse;
2222
use OCP\AppFramework\Http\RedirectResponse;
2323
use OCP\AppFramework\Http\TemplateResponse;
24+
use OCP\AppFramework\Services\IAppConfig;
2425
use OCP\AppFramework\Services\IInitialState;
2526
use OCP\IConfig;
2627
use OCP\IL10N;
@@ -32,10 +33,27 @@
3233

3334
class ConfigController extends Controller {
3435

36+
/**
37+
* The switches of the admin settings, and whether each one is stored lazily.
38+
* dashboard_enabled and issue_notifications_enabled are read in Application::register(),
39+
* which runs on every request, so they stay non-lazy.
40+
*/
41+
private const ADMIN_SWITCHES = [
42+
'use_popup' => true,
43+
'link_preview_enabled' => true,
44+
'allow_default_link_token_to_anonymous' => true,
45+
'allow_default_link_token_to_guests' => true,
46+
'dashboard_enabled' => false,
47+
'issue_notifications_enabled' => false,
48+
];
49+
50+
private const SECRET_KEYS = ['client_id', 'client_secret', 'default_link_token'];
51+
3552
public function __construct(
3653
string $appName,
3754
IRequest $request,
3855
private IConfig $config,
56+
private IAppConfig $appConfig,
3957
private IURLGenerator $urlGenerator,
4058
private IL10N $l,
4159
private IInitialState $initialStateService,
@@ -108,13 +126,15 @@ public function setConfig(array $values): DataResponse {
108126
* @return DataResponse
109127
*/
110128
public function setAdminConfig(array $values): DataResponse {
129+
// only the switches, so that a request cannot write a key the server owns, such as "enabled"
111130
foreach ($values as $key => $value) {
112-
if (in_array($key, ['client_id', 'client_secret', 'default_link_token'], true)) {
131+
if (!array_key_exists($key, self::ADMIN_SWITCHES) || !is_string($value)) {
113132
return new DataResponse([], Http::STATUS_BAD_REQUEST);
114-
} else {
115-
$this->config->setAppValue(Application::APP_ID, $key, $value);
116133
}
117134
}
135+
foreach ($values as $key => $value) {
136+
$this->appConfig->setAppValueString($key, $value, lazy: self::ADMIN_SWITCHES[$key]);
137+
}
118138
return new DataResponse([]);
119139
}
120140

@@ -127,14 +147,15 @@ public function setAdminConfig(array $values): DataResponse {
127147
#[PasswordConfirmationRequired]
128148
public function setSensitiveAdminConfig(array $values): DataResponse {
129149
foreach ($values as $key => $value) {
130-
if (in_array($key, ['client_id', 'client_secret', 'default_link_token'], true)) {
131-
$this->secretService->setEncryptedAppValue($key, $value);
132-
if ($key === 'default_link_token') {
133-
$info = $this->githubAPIService->updateLinkTokenUserInfo($value);
134-
return new DataResponse($info);
135-
}
136-
} else {
137-
$this->config->setAppValue(Application::APP_ID, $key, $value);
150+
if (!in_array($key, self::SECRET_KEYS, true) || !is_string($value)) {
151+
return new DataResponse([], Http::STATUS_BAD_REQUEST);
152+
}
153+
}
154+
foreach ($values as $key => $value) {
155+
$this->secretService->setEncryptedAppValue($key, $value);
156+
if ($key === 'default_link_token') {
157+
$info = $this->githubAPIService->updateLinkTokenUserInfo($value);
158+
return new DataResponse($info);
138159
}
139160
}
140161
return new DataResponse([]);
@@ -184,7 +205,7 @@ public function oauthRedirect(string $code, string $state): RedirectResponse {
184205
$this->config->setUserValue($this->userId, Application::APP_ID, 'token_type', 'oauth');
185206
$userInfo = $this->storeUserInfo();
186207

187-
$usePopup = $this->config->getAppValue(Application::APP_ID, 'use_popup', '0') === '1';
208+
$usePopup = $this->appConfig->getAppValueString('use_popup', '0', lazy: true) === '1';
188209
if ($usePopup) {
189210
return new RedirectResponse(
190211
$this->urlGenerator->linkToRoute('integration_github.config.popupSuccessPage', [

‎lib/Dashboard/GithubWidget.php‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -9,14 +9,14 @@
99

1010
use OCA\Github\AppInfo\Application;
1111
use OCA\Github\Service\GithubAPIService;
12+
use OCP\AppFramework\Services\IAppConfig;
1213
use OCP\AppFramework\Services\IInitialState;
1314
use OCP\Dashboard\IAPIWidget;
1415
use OCP\Dashboard\IButtonWidget;
1516
use OCP\Dashboard\IIconWidget;
1617
use OCP\Dashboard\IOptionWidget;
1718
use OCP\Dashboard\Model\WidgetButton;
1819
use OCP\Dashboard\Model\WidgetOptions;
19-
use OCP\IConfig;
2020
use OCP\IL10N;
2121
use OCP\IURLGenerator;
2222

@@ -26,7 +26,7 @@ class GithubWidget implements IAPIWidget, IButtonWidget, IIconWidget, IOptionWid
2626

2727
public function __construct(
2828
private IL10N $l10n,
29-
private IConfig $config,
29+
private IAppConfig $appConfig,
3030
private IURLGenerator $urlGenerator,
3131
private IInitialState $initialStateService,
3232
private GithubAPIService $githubAPIService,
@@ -82,10 +82,10 @@ public function getUrl(): ?string {
8282
* @inheritDoc
8383
*/
8484
public function load(): void {
85-
$clientID = $this->config->getAppValue(Application::APP_ID, 'client_id');
86-
$clientSecret = $this->config->getAppValue(Application::APP_ID, 'client_secret');
85+
$clientID = $this->appConfig->getAppValueString('client_id', lazy: true);
86+
$clientSecret = $this->appConfig->getAppValueString('client_secret', lazy: true);
8787
$oauthPossible = $clientID !== '' && $clientSecret !== '';
88-
$usePopup = $this->config->getAppValue(Application::APP_ID, 'use_popup', '0');
88+
$usePopup = $this->appConfig->getAppValueString('use_popup', '0', lazy: true);
8989

9090
$userConfig = [
9191
'oauth_is_possible' => $oauthPossible,

‎lib/Migration/Version030010Date20241002015812.php‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111
use Closure;
1212
use OCA\Github\AppInfo\Application;
1313
use OCP\DB\QueryBuilder\IQueryBuilder;
14-
use OCP\IConfig;
14+
use OCP\IAppConfig;
1515
use OCP\IDBConnection;
1616
use OCP\Migration\IOutput;
1717
use OCP\Migration\SimpleMigrationStep;
@@ -22,7 +22,7 @@ class Version030010Date20241002015812 extends SimpleMigrationStep {
2222
public function __construct(
2323
private IDBConnection $connection,
2424
private ICrypto $crypto,
25-
private IConfig $config,
25+
private IAppConfig $appConfig,
2626
) {
2727
}
2828

@@ -34,10 +34,10 @@ public function __construct(
3434
public function postSchemaChange(IOutput $output, Closure $schemaClosure, array $options) {
3535
// client_id, client_secret, default_link_token
3636
foreach (['client_id', 'client_secret', 'default_link_token'] as $key) {
37-
$value = $this->config->getAppValue(Application::APP_ID, $key);
37+
$value = $this->appConfig->getValueString(Application::APP_ID, $key, lazy: true);
3838
if ($value !== '') {
3939
$encryptedValue = $this->crypto->encrypt($value);
40-
$this->config->setAppValue(Application::APP_ID, $key, $encryptedValue);
40+
$this->appConfig->setValueString(Application::APP_ID, $key, $encryptedValue, lazy: true);
4141
}
4242
}
4343

‎lib/Migration/Version030301Date20261021102507.php‎

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
/**
6+
* SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors
7+
* SPDX-License-Identifier: AGPL-3.0-or-later
8+
*/
9+
10+
namespace OCA\Github\Migration;
11+
12+
use Closure;
13+
use OCA\Github\AppInfo\Application;
14+
use OCP\DB\ISchemaWrapper;
15+
use OCP\IAppConfig;
16+
use OCP\Migration\IOutput;
17+
use OCP\Migration\SimpleMigrationStep;
18+
19+
class Version030301Date20261021102507 extends SimpleMigrationStep {
20+
21+
/**
22+
* The app config keys that are only read by the app's own pages, settings, providers,
23+
* OAuth flow and API requests. dashboard_enabled and issue_notifications_enabled are
24+
* read on every request in Application::register() and stay non-lazy.
25+
*/
26+
private const LAZY_APP_CONFIG_KEYS = [
27+
'client_id',
28+
'client_secret',
29+
'default_link_token',
30+
'use_popup',
31+
'link_preview_enabled',
32+
'allow_default_link_token_to_anonymous',
33+
'allow_default_link_token_to_guests',
34+
'user_name',
35+
'user_displayname',
36+
];
37+
38+
public function __construct(
39+
private IAppConfig $appConfig,
40+
) {
41+
}
42+
43+
/**
44+
* @param IOutput $output
45+
* @param Closure $schemaClosure The `\Closure` returns a `ISchemaWrapper`
46+
* @param array $options
47+
*
48+
* @return null|ISchemaWrapper
49+
*/
50+
public function postSchemaChange(IOutput $output, Closure $schemaClosure, array $options) {
51+
// mark the values stored as non-lazy by earlier versions as lazy
52+
foreach (self::LAZY_APP_CONFIG_KEYS as $key) {
53+
$this->appConfig->updateLazy(Application::APP_ID, $key, true);
54+
}
55+
56+
return null;
57+
}
58+
}

‎lib/Reference/GithubCodeReferenceProvider.php‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
use OC\Collaboration\Reference\ReferenceManager;
1111
use OCA\Github\AppInfo\Application;
1212
use OCA\Github\Service\GithubAPIService;
13+
use OCP\AppFramework\Services\IAppConfig;
1314
use OCP\Collaboration\Reference\ADiscoverableReferenceProvider;
1415
use OCP\Collaboration\Reference\IReference;
1516
use OCP\Collaboration\Reference\Reference;
@@ -24,6 +25,7 @@ class GithubCodeReferenceProvider extends ADiscoverableReferenceProvider {
2425
public function __construct(
2526
private GithubAPIService $githubAPIService,
2627
private IConfig $config,
28+
private IAppConfig $appConfig,
2729
private IL10N $l10n,
2830
private IURLGenerator $urlGenerator,
2931
private ReferenceManager $referenceManager,
@@ -71,7 +73,7 @@ public function matchReference(string $referenceText): bool {
7173
return false;
7274
}
7375
}
74-
$adminLinkPreviewEnabled = $this->config->getAppValue(Application::APP_ID, 'link_preview_enabled', '1') === '1';
76+
$adminLinkPreviewEnabled = $this->appConfig->getAppValueString('link_preview_enabled', '1', lazy: true) === '1';
7577
if (!$adminLinkPreviewEnabled) {
7678
return false;
7779
}

‎lib/Reference/GithubIssuePrReferenceProvider.php‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
use OC\Collaboration\Reference\ReferenceManager;
1313
use OCA\Github\AppInfo\Application;
1414
use OCA\Github\Service\GithubAPIService;
15+
use OCP\AppFramework\Services\IAppConfig;
1516
use OCP\Collaboration\Reference\ADiscoverableReferenceProvider;
1617
use OCP\Collaboration\Reference\IReference;
1718
use OCP\Collaboration\Reference\ISearchableReferenceProvider;
@@ -31,6 +32,7 @@ class GithubIssuePrReferenceProvider extends ADiscoverableReferenceProvider impl
3132
public function __construct(
3233
private GithubAPIService $githubAPIService,
3334
private IConfig $config,
35+
private IAppConfig $appConfig,
3436
private IL10N $l10n,
3537
private IURLGenerator $urlGenerator,
3638
private ReferenceManager $referenceManager,
@@ -85,7 +87,7 @@ public function matchReference(string $referenceText): bool {
8587
return false;
8688
}
8789
}
88-
$adminLinkPreviewEnabled = $this->config->getAppValue(Application::APP_ID, 'link_preview_enabled', '1') === '1';
90+
$adminLinkPreviewEnabled = $this->appConfig->getAppValueString('link_preview_enabled', '1', lazy: true) === '1';
8991
if (!$adminLinkPreviewEnabled) {
9092
return false;
9193
}

‎lib/Service/GithubAPIService.php‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
use GuzzleHttp\Exception\ClientException;
1414
use GuzzleHttp\Exception\ServerException;
1515
use OCA\Github\AppInfo\Application;
16+
use OCP\AppFramework\Services\IAppConfig;
1617
use OCP\Dashboard\Model\WidgetItem;
1718
use OCP\Http\Client\IClient;
1819
use OCP\Http\Client\IClientService;
@@ -37,6 +38,7 @@ public function __construct(
3738
private LoggerInterface $logger,
3839
private IL10N $l10n,
3940
private IConfig $config,
41+
private IAppConfig $appConfig,
4042
private IURLGenerator $urlGenerator,
4143
private IUserManager $userManager,
4244
private IManager $notificationManager,
@@ -377,15 +379,15 @@ public function getCommitInfo(?string $userId, string $owner, string $repo, stri
377379
public function updateLinkTokenUserInfo(string $defaultLinkToken): array {
378380
$info = $this->request(null, 'user', [], 'GET', true, 30, $defaultLinkToken);
379381
if (isset($info['login'], $info['name'])) {
380-
$this->config->setAppValue(Application::APP_ID, 'user_name', $info['login']);
381-
$this->config->setAppValue(Application::APP_ID, 'user_displayname', $info['name']);
382+
$this->appConfig->setAppValueString('user_name', $info['login'], lazy: true);
383+
$this->appConfig->setAppValueString('user_displayname', $info['name'], lazy: true);
382384
return [
383385
'user_name' => $info['login'],
384386
'user_displayname' => $info['name'],
385387
];
386388
} else {
387-
$this->config->deleteAppValue(Application::APP_ID, 'user_name');
388-
$this->config->deleteAppValue(Application::APP_ID, 'user_displayname');
389+
$this->appConfig->deleteAppValue('user_name');
390+
$this->appConfig->deleteAppValue('user_displayname');
389391
return [
390392
'user_name' => '',
391393
'user_displayname' => '',
@@ -394,7 +396,7 @@ public function updateLinkTokenUserInfo(string $defaultLinkToken): array {
394396
}
395397

396398
public function checkNewNotifications() {
397-
$notificationEnabled = $this->config->getAppValue(Application::APP_ID, 'issue_notifications_enabled', '1') === '1';
399+
$notificationEnabled = $this->appConfig->getAppValueString('issue_notifications_enabled', '1') === '1';
398400
if (!$notificationEnabled) {
399401
return;
400402
}

0 commit comments

Comments
 (0)