fix(config): clear the old category cache on upgrade

The old cache is only replaced when the categories are requested, so
on instances that use a menu without categories the 170 KB app config
value stayed and was still loaded on every request. A post-migration
repair step clears it when the app is upgraded.

CHANGELOG.md

@@ -4,6 +4,7 @@
 
 - fix(config): store the app store category cache lazily with only the names in the languages in use, 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
+- fix(config): remove the old category cache from the app config on upgrade
 
 ## 6.2.0
 

appinfo/info.xml

@@ -55,6 +55,11 @@ In case of downtime, you can download **Custom Menu** from [here](https://kim.de
         <php min-version="8.1" max-version="8.5" />
         <nextcloud min-version="34" max-version="35"/>
     </dependencies>
+    <repair-steps>
+        <post-migration>
+            <step>OCA\SideMenu\Migration\ClearCategoryCache</step>
+        </post-migration>
+    </repair-steps>
     <settings>
         <admin>OCA\SideMenu\Settings\Admin</admin>
         <admin-section>OCA\SideMenu\Settings\AdminSection</admin-section>

lib/AppInfo/Application.php

@@ -21,6 +21,7 @@ use OCP\AppFramework\Bootstrap\IRegistrationContext;
 use OCP\AppFramework\Utility\ITimeFactory;
 use OCP\EventDispatcher\IEventDispatcher;
 use OCP\IAppConfig;
+use OCP\ICacheFactory;
 use OCP\IConfig;
 use OCP\INavigationManager;
 use OCP\IRequest;
@@ -62,7 +63,8 @@ class Application extends App implements IBootstrap
                 $c->get(IUserSession::class),
                 $c->get(IAppConfig::class),
                 $c->get(ITimeFactory::class),
-                $c->get(LangRepository::class)
+                $c->get(LangRepository::class),
+                $c->get(ICacheFactory::class)
             );
         });
 

lib/Migration/ClearCategoryCache.php

@@ -0,0 +1,46 @@
+<?php
+
+/**
+ * @license GNU AGPL version 3 or any later version
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License as
+ * published by the Free Software Foundation, either version 3 of the
+ * License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Affero General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+namespace OCA\SideMenu\Migration;
+
+use OCA\SideMenu\Service\CategoryRepository;
+use OCP\Migration\IOutput;
+use OCP\Migration\IRepairStep;
+
+/**
+ * Earlier versions stored the category cache as a regular app config
+ * value, which Nextcloud loads on every request. Clear it on upgrade so
+ * it does not linger on instances that never request the categories.
+ */
+class ClearCategoryCache implements IRepairStep
+{
+    public function __construct(protected CategoryRepository $categoryRepository)
+    {
+    }
+
+    public function getName(): string
+    {
+        return 'Clear the category cache';
+    }
+
+    public function run(IOutput $output): void
+    {
+        $this->categoryRepository->clearCache();
+    }
+}

lib/Service/CategoryRepository.php

@@ -5,7 +5,10 @@ namespace OCA\SideMenu\Service;
 use OC\App\AppStore\Fetcher\CategoryFetcher;
 use OCA\SideMenu\AppInfo\Application;
 use OCP\AppFramework\Utility\ITimeFactory;
+use OCP\Exceptions\AppConfigTypeConflictException;
 use OCP\IAppConfig;
+use OCP\ICache;
+use OCP\ICacheFactory;
 use OCP\IConfig;
 use OCP\IUserSession;
 use OCP\L10N\IFactory;
@@ -33,6 +36,7 @@ class CategoryRepository
         protected IAppConfig $appConfig,
         protected ITimeFactory $timeFactory,
         protected LangRepository $langRepository,
+        protected ICacheFactory $cacheFactory,
     ) {
     }
 
@@ -94,6 +98,7 @@ class CategoryRepository
 
     public function clearCache(): void
     {
+        $this->getDistributedCache()?->remove(self::CACHE_KEY);
         $this->appConfig->deleteKey(Application::APP_ID, self::CACHE_KEY);
     }
 
@@ -117,18 +122,57 @@ class CategoryRepository
     }
 
     /**
-     * Returns the app store categories with only their names, from a
+     * Returns the app store categories with only their names. They are
-     * cache that is stored lazily: it holds the names in every language,
+     * cached in the distributed cache (e.g. Redis) when one is configured,
-     * so it must not be loaded on every request with the other settings.
+     * otherwise in a lazy app config value, so they are not loaded on
+     * every request with the other settings.
      */
     protected function getCategories(): array
     {
-        $cache = $this->appConfig->getValueArray(Application::APP_ID, self::CACHE_KEY, [], lazy: true);
+        $cache = $this->getDistributedCache();
 
-        if (isset($cache['expires'], $cache['categories']) && $cache['expires'] > $this->timeFactory->getTime()) {
+        if ($cache !== null) {
-            return $cache['categories'];
+            $categories = $cache->get(self::CACHE_KEY);
+
+            if (is_array($categories)) {
+                return $categories;
+            }
+        } else {
+            try {
+                $stored = $this->appConfig->getValueArray(Application::APP_ID, self::CACHE_KEY, [], lazy: true);
+            } catch (AppConfigTypeConflictException) {
+                // Stored with another type, e.g. set by hand; refreshed below
+                $stored = [];
+            }
+
+            if (isset($stored['expires'], $stored['categories']) && $stored['expires'] > $this->timeFactory->getTime()) {
+                return $stored['categories'];
+            }
         }
 
+        $categories = $this->fetchCategories();
+
+        // Don't cache an empty list, e.g. when the app store is unreachable
+        if (empty($categories)) {
+            return $categories;
+        }
+
+        $this->clearCache();
+
+        if ($cache !== null) {
+            $cache->set(self::CACHE_KEY, $categories, self::CACHE_TTL);
+        } else {
+            $this->appConfig->setValueArray(Application::APP_ID, self::CACHE_KEY, [
+                'expires' => $this->timeFactory->getTime() + self::CACHE_TTL,
+                'categories' => $categories,
+            ], lazy: true);
+        }
+
+        return $categories;
+    }
+
+    protected function fetchCategories(): array
+    {
         $langs = $this->getCachedLangs();
         $categories = [];
 
@@ -147,15 +191,13 @@ class CategoryRepository
             ];
         }
 
-        // 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;
     }
+
+    protected function getDistributedCache(): ?ICache
+    {
+        return $this->cacheFactory->isAvailable()
+            ? $this->cacheFactory->createDistributed(Application::APP_ID)
+            : null;
+    }
 }