From ccefefa6f7ee47d708f2a54c3f4f70982d7b0d4c Mon Sep 17 00:00:00 2001 From: Dillard Blom Date: Wed, 30 Sep 2026 12:01:50 +0200 Subject: [PATCH] fix(config): keep the category cache small and save settings at once The app store category cache (cache-categories) held the name and description of every category in every language, about 170 KB, stored as a regular app config value. Nextcloud loads all regular values of all apps on every request, so this one value was read on every page load. It was only refreshed with the "remove cache" button. - Keep only the category ids and names, store the cache as a lazy value so it is only loaded when categories are needed, and refresh it after a week. - Only load the categories where they are used: the menus with categories, and the app list when custom categories are set. - Save all settings in one request to a new admin endpoint, which accepts only known settings, then clears the category cache and changes the stylesheet cache key. The settings page sent one request per setting and reloaded right after; with Nextcloud's short-lived local config cache, the reloaded page could show a half-saved configuration, so a change sometimes had to be made twice. --- CHANGELOG.md | 5 + lib/AppInfo/Application.php | 6 +- lib/Controller/AdminSettingController.php | 121 +++++++++++++------- lib/Service/AppRepository.php | 3 +- lib/Service/CategoryRepository.php | 64 +++++++++-- src/components/settings/AdminSaveButton.vue | 59 +++------- 6 files changed, 169 insertions(+), 89 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index eaf203b..5aff943 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,10 @@ ## [Unreleased] +### Fixed + +- fix(config): store the app store category cache lazily with only the names, refresh it weekly and when the settings are saved, and load it only when categories are used +- fix(admin): save all settings in one request so changes are not lost after the page reloads + ## 6.2.0 ### Added diff --git a/lib/AppInfo/Application.php b/lib/AppInfo/Application.php index 2da9429..2f6b936 100644 --- a/lib/AppInfo/Application.php +++ b/lib/AppInfo/Application.php @@ -17,7 +17,9 @@ use OCP\AppFramework\App; use OCP\AppFramework\Bootstrap\IBootContext; use OCP\AppFramework\Bootstrap\IBootstrap; use OCP\AppFramework\Bootstrap\IRegistrationContext; +use OCP\AppFramework\Utility\ITimeFactory; use OCP\EventDispatcher\IEventDispatcher; +use OCP\IAppConfig; use OCP\IConfig; use OCP\INavigationManager; use OCP\IRequest; @@ -56,7 +58,9 @@ class Application extends App implements IBootstrap $c->get(ConfigProxy::class), $c->get(IConfig::class), $c->get(IFactory::class), - $c->get(IUserSession::class) + $c->get(IUserSession::class), + $c->get(IAppConfig::class), + $c->get(ITimeFactory::class) ); }); diff --git a/lib/Controller/AdminSettingController.php b/lib/Controller/AdminSettingController.php index 42f6cd2..82c1705 100644 --- a/lib/Controller/AdminSettingController.php +++ b/lib/Controller/AdminSettingController.php @@ -20,12 +20,14 @@ namespace OCA\SideMenu\Controller; use OCA\SideMenu\AppInfo\Application; +use OCA\SideMenu\Service\CategoryRepository; use OCA\SideMenu\Service\Color; use OCA\SideMenu\Service\ConfigProxy; use OCA\SideMenu\Service\LangRepository; use OCP\AppFramework\Controller; use OCP\AppFramework\Http\Attribute\FrontpageRoute; use OCP\AppFramework\Http\Attribute\NoCSRFRequired; +use OCP\AppFramework\Http\Attribute\PasswordConfirmationRequired; use OCP\AppFramework\Http\DataDownloadResponse; use OCP\AppFramework\Http\JSONResponse; use OCP\AppFramework\Http\RedirectResponse; @@ -35,6 +37,12 @@ use OCP\IURLGenerator; class AdminSettingController extends Controller { + /** + * Keys that are managed by the app or by Nextcloud itself ("enabled") + * and must not be written from the settings form. + */ + protected const UNSAVED_KEYS = ['cache', 'cache-categories', 'enabled']; + public function __construct( $appName, IRequest $request, @@ -43,6 +51,7 @@ class AdminSettingController extends Controller protected IURLGenerator $urlGenerator, protected Color $color, protected LangRepository $langRepository, + protected CategoryRepository $categoryRepository, ) { parent::__construct($appName, $request); } @@ -51,7 +60,7 @@ class AdminSettingController extends Controller #[FrontpageRoute(verb: 'GET', url: '/admin/cache/remove')] public function removeCache(): RedirectResponse { - $this->config->setAppValue(Application::APP_ID, 'cache-categories', '[]'); + $this->categoryRepository->clearCache(); return new RedirectResponse($this->urlGenerator->linkToRoute('settings.AdminSettings.index', [ 'section' => Application::APP_ID, @@ -131,7 +140,77 @@ class AdminSettingController extends Controller 'top-menu-mouse-over-hidden-label', ]; - $defaults = [ + $defaults = $this->getDefaults(); + + $config = []; + + foreach ($keys as $key) { + if (!isset($defaults[$key])) { + continue; + } + + if (in_array($key, $booleans)) { + $config[$key] = $this->configProxy->getAppValueBool($key, $defaults[$key]); + } elseif (in_array($key, $arrays)) { + $config[$key] = $this->configProxy->getAppValueArray($key, $defaults[$key]); + } elseif (in_array($key, $integers)) { + $config[$key] = $this->configProxy->getAppValueInt($key, $defaults[$key]); + } else { + $config[$key] = $this->configProxy->getAppValue($key, $defaults[$key]); + } + } + + foreach ($defaults as $key => $default) { + if (array_key_exists($key, $config)) { + continue; + } + + if (in_array($key, $booleans)) { + $config[$key] = $default === '1'; + } elseif (in_array($key, $arrays)) { + $config[$key] = json_decode($default); + } elseif (in_array($key, $integers)) { + $config[$key] = (int) $default; + } else { + $config[$key] = $default; + } + } + + $config['langs'] = $this->langRepository->getUsedLangs(); + + return new JSONResponse($config); + } + + #[PasswordConfirmationRequired] + #[FrontpageRoute(verb: 'POST', url: '/admin/config')] + public function saveConfiguration(array $config): JSONResponse + { + $allowed = array_diff(array_keys($this->getDefaults()), self::UNSAVED_KEYS); + + foreach ($config as $key => $value) { + if (!in_array($key, $allowed, true)) { + continue; + } + + if (is_array($value)) { + $value = json_encode($value); + } elseif (is_bool($value)) { + $value = $value ? '1' : '0'; + } + + $this->config->setAppValue(Application::APP_ID, $key, (string) $value); + } + + // New stylesheet URL, and category names that match the new settings + $this->config->setAppValue(Application::APP_ID, 'cache', (string) time()); + $this->categoryRepository->clearCache(); + + return new JSONResponse(['status' => 'ok']); + } + + protected function getDefaults(): array + { + return [ 'opener-only' => '0', 'opener-hover' => '0', 'display-logo' => '1', @@ -188,43 +267,5 @@ class AdminSettingController extends Controller 'categories-order-type' => 'default', ]; - - $config = []; - - foreach ($keys as $key) { - if (!isset($defaults[$key])) { - continue; - } - - if (in_array($key, $booleans)) { - $config[$key] = $this->configProxy->getAppValueBool($key, $defaults[$key]); - } elseif (in_array($key, $arrays)) { - $config[$key] = $this->configProxy->getAppValueArray($key, $defaults[$key]); - } elseif (in_array($key, $integers)) { - $config[$key] = $this->configProxy->getAppValueInt($key, $defaults[$key]); - } else { - $config[$key] = $this->configProxy->getAppValue($key, $defaults[$key]); - } - } - - foreach ($defaults as $key => $default) { - if (array_key_exists($key, $config)) { - continue; - } - - if (in_array($key, $booleans)) { - $config[$key] = $default === '1'; - } elseif (in_array($key, $arrays)) { - $config[$key] = json_decode($default); - } elseif (in_array($key, $integers)) { - $config[$key] = (int) $default; - } else { - $config[$key] = $default; - } - } - - $config['langs'] = $this->langRepository->getUsedLangs(); - - return new JSONResponse($config); } } diff --git a/lib/Service/AppRepository.php b/lib/Service/AppRepository.php index 3c64f7d..b66a587 100644 --- a/lib/Service/AppRepository.php +++ b/lib/Service/AppRepository.php @@ -40,7 +40,8 @@ class AppRepository { $navigation = $this->navigationManager->getAll(); $appCategoriesCustom = $this->config->getAppValueArray('apps-categories-custom', '[]'); - $categories = $this->categoryRepository->getOrderedCategories(); + // Only needed to check custom categories, skip loading them otherwise + $categories = empty(array_filter($appCategoriesCustom)) ? [] : $this->categoryRepository->getOrderedCategories(); $apps = $this->ocApp->listAllApps(); $visibleApps = []; diff --git a/lib/Service/CategoryRepository.php b/lib/Service/CategoryRepository.php index 404c061..d79dbcd 100644 --- a/lib/Service/CategoryRepository.php +++ b/lib/Service/CategoryRepository.php @@ -4,6 +4,8 @@ namespace OCA\SideMenu\Service; use OC\App\AppStore\Fetcher\CategoryFetcher; use OCA\SideMenu\AppInfo\Application; +use OCP\AppFramework\Utility\ITimeFactory; +use OCP\IAppConfig; use OCP\IConfig; use OCP\IUserSession; use OCP\L10N\IFactory; @@ -15,12 +17,21 @@ use OCP\L10N\IFactory; */ class CategoryRepository { + public const CACHE_KEY = 'cache-categories'; + + /** + * The app store categories rarely change, refresh them once a week. + */ + protected const CACHE_TTL = 604800; + public function __construct( protected CategoryFetcher $categoryFetcher, protected ConfigProxy $config, protected IConfig $iConfig, protected IFactory $l10nFactory, protected IUserSession $userSession, + protected IAppConfig $appConfig, + protected ITimeFactory $timeFactory, ) { } @@ -32,15 +43,9 @@ class CategoryRepository $currentLanguage = substr($this->l10nFactory->findLanguage(), 0, 2); $type = $this->config->getAppValue('categories-order-type', 'default'); $order = $this->config->getAppValueArray('categories-order', '[]'); - $categoriesLabels = $this->config->getAppValueArray('cache-categories', '[]'); + $categoriesLabels = $this->getCategories(); $customCategories = $this->config->getAppValueArray('categories-custom', '[]'); - if (empty($categoriesLabels)) { - $categoriesLabels = $this->categoryFetcher->get(); - - $this->iConfig->setAppValue(Application::APP_ID, 'cache-categories', json_encode($categoriesLabels)); - } - foreach ($categoriesLabels as $k => $category) { $categoriesLabels[$category['id']] = $category['translations'][$currentLanguage]['name'] ?? $category['translations']['en']['name']; @@ -85,4 +90,49 @@ class CategoryRepository return $categoriesLabels; } + + public function clearCache(): void + { + $this->appConfig->deleteKey(Application::APP_ID, self::CACHE_KEY); + } + + /** + * Returns the app store categories with only their names, from a + * cache that is stored lazily: it holds the names in every language, + * so it must not be loaded on every request with the other settings. + */ + protected function getCategories(): array + { + $cache = $this->appConfig->getValueArray(Application::APP_ID, self::CACHE_KEY, [], lazy: true); + + if (isset($cache['expires'], $cache['categories']) && $cache['expires'] > $this->timeFactory->getTime()) { + return $cache['categories']; + } + + $categories = []; + + foreach ($this->categoryFetcher->get() as $category) { + $names = []; + + foreach ($category['translations'] ?? [] as $lang => $translation) { + $names[$lang] = ['name' => $translation['name'] ?? '']; + } + + $categories[] = [ + 'id' => $category['id'], + 'translations' => $names, + ]; + } + + // Don't cache an empty list, e.g. when the app store is unreachable + if (!empty($categories)) { + $this->clearCache(); + $this->appConfig->setValueArray(Application::APP_ID, self::CACHE_KEY, [ + 'expires' => $this->timeFactory->getTime() + self::CACHE_TTL, + 'categories' => $categories, + ], lazy: true); + } + + return $categories; + } } diff --git a/src/components/settings/AdminSaveButton.vue b/src/components/settings/AdminSaveButton.vue index cd639a8..f3c1f7d 100644 --- a/src/components/settings/AdminSaveButton.vue +++ b/src/components/settings/AdminSaveButton.vue @@ -20,6 +20,9 @@