Repository navigation
perf: load the app config values lazily - #265
Open
oleksandr-nc wants to merge 1 commit into
Open
oleksandr-nc wants to merge 1 commit into
oleksandr-nc wants to merge 1 commit into
Conversation
oleksandr-nc
force-pushed
the
perf/lazy-app-config
branch
5 times, most recently
from
October 21, 2026 12:30
57ff8de to
bdc8d9f
Compare
Since Nextcloud 29 every request loads the non-lazy app config rows of every app. All of this app's values were read and written through the deprecated IConfig::getAppValue() and setAppValue(), and setAppValue() always stores a key as non-lazy, although only dashboard_enabled and issue_notifications_enabled are read on every request, in Application::register(). Every read and write of an app config key now goes through the typed methods of IAppConfig, and every delete through its deleteAppValue(): the app-scoped IAppConfig in the app's code, and OCP\IAppConfig in the two migrations. The OAuth client id and secret, the default link token, use_popup, link_preview_enabled, the two allow_default_link_token_* switches and the default token's user_name and user_displayname pass lazy: true on every read and write, because a read without it can return the default once the key is lazy, and a write without it makes the key non-lazy again. dashboard_enabled and issue_notifications_enabled stay non-lazy. Stored values keep their format, and the secrets stay encrypted with ICrypto. Besides the three secrets, the two admin endpoints wrote any key the request carried. Written lazily, the server's own enabled key would make the app look disabled, so setAdminConfig() now accepts only the six switches of the admin settings and setSensitiveAdminConfig() only the three secrets, both only as strings, and they answer 400 without writing anything otherwise. A new migration marks the lazy keys lazy on existing installs with IAppConfig::updateLazy(). The unit tests pin the flag and the default of each call, which of the app's keys, the server's own keys and an unknown key each endpoint accepts, and the keys the migration marks. Signed-off-by: Oleksander Piskun <oleksandr2088@icloud.com>
oleksandr-nc
force-pushed
the
perf/lazy-app-config
branch
from
October 21, 2026 12:53
bdc8d9f to
f6dd915
Compare
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Since Nextcloud 29 every request loads the non-lazy app config values of every app, so an app should keep non-lazy only what it reads on requests that have nothing to do with it.
Every value was loaded on every request
All values were read and written through the deprecated
IConfig::getAppValue()andsetAppValue(), andsetAppValue()always stores a key as non-lazy. The OAuth client credentials, the default link token and the admin switches were therefore loaded on every request of every user, although onlydashboard_enabledandissue_notifications_enabledare read there, inApplication::register().Only the two registration switches stay non-lazy
Every read and write of an app config key now goes through the typed methods of
IAppConfig, and every delete through itsdeleteAppValue(): the app-scopedIAppConfigin the app's code, andOCP\IAppConfigin the two migrations,Version030010Date20241002015812, which encrypted the stored credentials, and the new one. NogetAppValue()orsetAppValue()call is left. The lazy keys passlazy: trueon every read and write, because a read without it can return the default once the key is lazy, and a write without it makes the key non-lazy again.Lazy, read only by the app's own settings, dashboard widget, OAuth flow, reference providers and GitHub requests:
client_id,client_secret: the OAuth app, read by both settings pages, the dashboard widget, the OAuth redirect and the token revocationdefault_link_token,allow_default_link_token_to_anonymous,allow_default_link_token_to_guests: read by the admin settings and bySecretService::getAccessToken()user_name,user_displayname: the GitHub account of the default token, written when the token is saved or when the admin settings find them missing, and shown thereuse_popup: read by both settings pages, the dashboard widget and the OAuth redirectlink_preview_enabled: read by the two reference providers whenever a link is resolved, and by the admin settingsNon-lazy:
dashboard_enabled: decides inApplication::register()whether the dashboard widget is registered, and is also read by the admin settingsissue_notifications_enabled: decides inApplication::register()whether the notifier is registered, and is also read by the notification job and both settings pagesStored values keep their format (
'1'and'0'strings), and the client credentials and the default token stay encrypted withICrypto.The admin endpoints only write the settings they are for
Besides the three secrets,
setAdminConfig()andsetSensitiveAdminConfig()wrote any key the request carried. Written lazily, a key the server owns would break the app: a lazyenabledrow is not found by the app manager, and the app looks disabled.setAdminConfig()now accepts only the six switches of the admin settings,setSensitiveAdminConfig()onlyclient_id,client_secretanddefault_link_token, both only as strings, and both answer 400 without writing anything otherwise. The admin settings page sends nothing else.Existing installs mark the keys lazy on upgrade
Version030301Date20261021102507callsIAppConfig::updateLazy()for each lazy key, which leaves keys that were never set alone. It runs with the next version bump. Until then, for example on a checkout ofmain, reads withlazy: truestill find the values stored non-lazy, and the next write of a key stores it lazy.馃 AI (if applicable)