Merge pull request #52793 from nextcloud/feat/cache-routes

16 files changed, 192 insertions, 47 deletions

diff --git a/.github/workflows/integration-sqlite.yml b/.github/workflows/integration-sqlite.yml
index b067ff75674..e578ee2b6e3 100644
--- a/.github/workflows/integration-sqlite.yml
+++ b/.github/workflows/integration-sqlite.yml
@@ -163,7 +163,7 @@ jobs:
       - name: Print logs
         if: always()
         run: |
-          cat data/nextcloud.log
+          cat $(./occ log:file |grep "Log file"|cut -d" " -f3)
           docker ps -a
           docker ps -aq | while read container ; do IMAGE=$(docker inspect --format='{{.Config.Image}}' $container); echo $IMAGE; docker logs $container; echo "\n\n" ; done
 
diff --git a/build/psalm-baseline.xml b/build/psalm-baseline.xml
index 9b70702914c..39eeb1e91d1 100644
--- a/build/psalm-baseline.xml
+++ b/build/psalm-baseline.xml
@@ -4336,9 +4336,6 @@
     </ParamNameMismatch>
   </file>
   <file src="lib/private/Route/Router.php">
-    <InvalidClass>
-      <code><![CDATA[\OC_APP]]></code>
-    </InvalidClass>
     <InvalidNullableReturnType>
       <code><![CDATA[string]]></code>
     </InvalidNullableReturnType>
diff --git a/lib/private/App/AppManager.php b/lib/private/App/AppManager.php
index 18f29114d0e..4223d09e3dc 100644
--- a/lib/private/App/AppManager.php
+++ b/lib/private/App/AppManager.php
@@ -18,6 +18,7 @@ use OCP\Collaboration\AutoComplete\IManager as IAutoCompleteManager;
 use OCP\Collaboration\Collaborators\ISearch as ICollaboratorSearch;
 use OCP\Diagnostics\IEventLogger;
 use OCP\EventDispatcher\IEventDispatcher;
+use OCP\IAppConfig;
 use OCP\ICacheFactory;
 use OCP\IConfig;
 use OCP\IGroup;
@@ -134,7 +135,8 @@ class AppManager implements IAppManager {
 	 */
 	private function getEnabledAppsValues(): array {
 		if (!$this->enabledAppsCache) {
-			$values = $this->getAppConfig()->getValues(false, 'enabled');
+			/** @var array<string,string> */
+			$values = $this->getAppConfig()->searchValues('enabled', false, IAppConfig::VALUE_STRING);
 
 			$alwaysEnabledApps = $this->getAlwaysEnabledApps();
 			foreach ($alwaysEnabledApps as $appId) {
@@ -784,8 +786,8 @@ class AppManager implements IAppManager {
 	 *
 	 * @return array<string, string>
 	 */
-	public function getAppInstalledVersions(): array {
-		return $this->getAppConfig()->getAppInstalledVersions();
+	public function getAppInstalledVersions(bool $onlyEnabled = false): array {
+		return $this->getAppConfig()->getAppInstalledVersions($onlyEnabled);
 	}
 
 	/**
diff --git a/lib/private/AppConfig.php b/lib/private/AppConfig.php
index a8a6f689ffa..adbfc58978b 100644
--- a/lib/private/AppConfig.php
+++ b/lib/private/AppConfig.php
@@ -1670,11 +1670,18 @@ class AppConfig implements IAppConfig {
 	 *
 	 * @return array<string, string>
 	 */
-	public function getAppInstalledVersions(): array {
+	public function getAppInstalledVersions(bool $onlyEnabled = false): array {
 		if ($this->appVersionsCache === null) {
 			/** @var array<string, string> */
 			$this->appVersionsCache = $this->searchValues('installed_version', false, IAppConfig::VALUE_STRING);
 		}
+		if ($onlyEnabled) {
+			return array_filter(
+				$this->appVersionsCache,
+				fn (string $app): bool => $this->getValueString($app, 'enabled', 'no') !== 'no',
+				ARRAY_FILTER_USE_KEY
+			);
+		}
 		return $this->appVersionsCache;
 	}
 }
diff --git a/lib/private/AppFramework/Services/AppConfig.php b/lib/private/AppFramework/Services/AppConfig.php
index 77c5ea4de0c..04d97738483 100644
--- a/lib/private/AppFramework/Services/AppConfig.php
+++ b/lib/private/AppFramework/Services/AppConfig.php
@@ -343,7 +343,7 @@ class AppConfig implements IAppConfig {
 	 *
 	 * @return array<string, string>
 	 */
-	public function getAppInstalledVersions(): array {
-		return $this->appConfig->getAppInstalledVersions();
+	public function getAppInstalledVersions(bool $onlyEnabled = false): array {
+		return $this->appConfig->getAppInstalledVersions($onlyEnabled);
 	}
 }
diff --git a/lib/private/Route/CachingRouter.php b/lib/private/Route/CachingRouter.php
index 7dd26827d3c..dbd5ef02603 100644
--- a/lib/private/Route/CachingRouter.php
+++ b/lib/private/Route/CachingRouter.php
@@ -15,10 +15,16 @@ use OCP\IConfig;
 use OCP\IRequest;
 use Psr\Container\ContainerInterface;
 use Psr\Log\LoggerInterface;
+use Symfony\Component\Routing\Exception\ResourceNotFoundException;
+use Symfony\Component\Routing\Matcher\CompiledUrlMatcher;
+use Symfony\Component\Routing\Matcher\Dumper\CompiledUrlMatcherDumper;
+use Symfony\Component\Routing\RouteCollection;
 
 class CachingRouter extends Router {
 	protected ICache $cache;
 
+	protected array $legacyCreatedRoutes = [];
+
 	public function __construct(
 		ICacheFactory $cacheFactory,
 		LoggerInterface $logger,
@@ -54,4 +60,98 @@ class CachingRouter extends Router {
 			return $url;
 		}
 	}
+
+	private function serializeRouteCollection(RouteCollection $collection): array {
+		$dumper = new CompiledUrlMatcherDumper($collection);
+		return $dumper->getCompiledRoutes();
+	}
+
+	/**
+	 * Find the route matching $url
+	 *
+	 * @param string $url The url to find
+	 * @throws \Exception
+	 * @return array
+	 */
+	public function findMatchingRoute(string $url): array {
+		$this->eventLogger->start('cacheroute:match', 'Match route');
+		$key = $this->context->getHost() . '#' . $this->context->getBaseUrl() . '#rootCollection';
+		$cachedRoutes = $this->cache->get($key);
+		if (!$cachedRoutes) {
+			parent::loadRoutes();
+			$cachedRoutes = $this->serializeRouteCollection($this->root);
+			$this->cache->set($key, $cachedRoutes, 3600);
+		}
+		$matcher = new CompiledUrlMatcher($cachedRoutes, $this->context);
+		$this->eventLogger->start('cacheroute:url:match', 'Symfony URL match call');
+		try {
+			$parameters = $matcher->match($url);
+		} catch (ResourceNotFoundException $e) {
+			if (!str_ends_with($url, '/')) {
+				// We allow links to apps/files? for backwards compatibility reasons
+				// However, since Symfony does not allow empty route names, the route
+				// we need to match is '/', so we need to append the '/' here.
+				try {
+					$parameters = $matcher->match($url . '/');
+				} catch (ResourceNotFoundException $newException) {
+					// If we still didn't match a route, we throw the original exception
+					throw $e;
+				}
+			} else {
+				throw $e;
+			}
+		}
+		$this->eventLogger->end('cacheroute:url:match');
+
+		$this->eventLogger->end('cacheroute:match');
+		return $parameters;
+	}
+
+	/**
+	 * @param array{action:mixed, ...} $parameters
+	 */
+	protected function callLegacyActionRoute(array $parameters): void {
+		/*
+		 * Closures cannot be serialized to cache, so for legacy routes calling an action we have to include the routes.php file again
+		 */
+		$app = $parameters['app'];
+		$this->useCollection($app);
+		parent::requireRouteFile($parameters['route-file'], $app);
+		$collection = $this->getCollection($app);
+		$parameters['action'] = $collection->get($parameters['_route'])?->getDefault('action');
+		parent::callLegacyActionRoute($parameters);
+	}
+
+	/**
+	 * Create a \OC\Route\Route.
+	 * Deprecated
+	 *
+	 * @param string $name Name of the route to create.
+	 * @param string $pattern The pattern to match
+	 * @param array $defaults An array of default parameter values
+	 * @param array $requirements An array of requirements for parameters (regexes)
+	 */
+	public function create($name, $pattern, array $defaults = [], array $requirements = []): Route {
+		$this->legacyCreatedRoutes[] = $name;
+		return parent::create($name, $pattern, $defaults, $requirements);
+	}
+
+	/**
+	 * Require a routes.php file
+	 */
+	protected function requireRouteFile(string $file, string $appName): void {
+		$this->legacyCreatedRoutes = [];
+		parent::requireRouteFile($file, $appName);
+		foreach ($this->legacyCreatedRoutes as $routeName) {
+			$route = $this->collection?->get($routeName);
+			if ($route === null) {
+				/* Should never happen */
+				throw new \Exception("Could not find route $routeName");
+			}
+			if ($route->hasDefault('action')) {
+				$route->setDefault('route-file', $file);
+				$route->setDefault('app', $appName);
+			}
+		}
+	}
 }
diff --git a/lib/private/Route/Route.php b/lib/private/Route/Route.php
index ab5a1f6b59a..08231649e76 100644
--- a/lib/private/Route/Route.php
+++ b/lib/private/Route/Route.php
@@ -124,15 +124,9 @@ class Route extends SymfonyRoute implements IRoute {
 	 * The action to execute when this route matches, includes a file like
 	 * it is called directly
 	 * @param string $file
-	 * @return void
 	 */
 	public function actionInclude($file) {
-		$function = function ($param) use ($file) {
-			unset($param['_route']);
-			$_GET = array_merge($_GET, $param);
-			unset($param);
-			require_once "$file";
-		} ;
-		$this->action($function);
+		$this->setDefault('file', $file);
+		return $this;
 	}
 }
diff --git a/lib/private/Route/Router.php b/lib/private/Route/Router.php
index 376852a1b6e..02f371e808a 100644
--- a/lib/private/Route/Router.php
+++ b/lib/private/Route/Router.php
@@ -82,7 +82,7 @@ class Router implements IRouter {
 	public function getRoutingFiles() {
 		if ($this->routingFiles === null) {
 			$this->routingFiles = [];
-			foreach (\OC_APP::getEnabledApps() as $app) {
+			foreach ($this->appManager->getEnabledApps() as $app) {
 				try {
 					$appPath = $this->appManager->getAppPath($app);
 					$file = $appPath . '/appinfo/routes.php';
@@ -117,7 +117,7 @@ class Router implements IRouter {
 			$routingFiles = $this->getRoutingFiles();
 
 			$this->eventLogger->start('route:load:attributes', 'Loading Routes from attributes');
-			foreach (\OC_App::getEnabledApps() as $enabledApp) {
+			foreach ($this->appManager->getEnabledApps() as $enabledApp) {
 				$this->loadAttributeRoutes($enabledApp);
 			}
 			$this->eventLogger->end('route:load:attributes');
@@ -312,17 +312,11 @@ class Router implements IRouter {
 			$application = $this->getApplicationClass($caller[0]);
 			\OC\AppFramework\App::main($caller[1], $caller[2], $application->getContainer(), $parameters);
 		} elseif (isset($parameters['action'])) {
-			$action = $parameters['action'];
-			if (!is_callable($action)) {
-				throw new \Exception('not a callable action');
-			}
-			unset($parameters['action']);
-			unset($parameters['caller']);
-			$this->eventLogger->start('route:run:call', 'Run callable route');
-			call_user_func($action, $parameters);
-			$this->eventLogger->end('route:run:call');
+			$this->logger->warning('Deprecated action route used', ['parameters' => $parameters]);
+			$this->callLegacyActionRoute($parameters);
 		} elseif (isset($parameters['file'])) {
-			include $parameters['file'];
+			$this->logger->debug('Deprecated file route used', ['parameters' => $parameters]);
+			$this->includeLegacyFileRoute($parameters);
 		} else {
 			throw new \Exception('no action available');
 		}
@@ -330,6 +324,32 @@ class Router implements IRouter {
 	}
 
 	/**
+	 * @param array{file:mixed, ...} $parameters
+	 */
+	protected function includeLegacyFileRoute(array $parameters): void {
+		$param = $parameters;
+		unset($param['_route']);
+		$_GET = array_merge($_GET, $param);
+		unset($param);
+		require_once $parameters['file'];
+	}
+
+	/**
+	 * @param array{action:mixed, ...} $parameters
+	 */
+	protected function callLegacyActionRoute(array $parameters): void {
+		$action = $parameters['action'];
+		if (!is_callable($action)) {
+			throw new \Exception('not a callable action');
+		}
+		unset($parameters['action']);
+		unset($parameters['caller']);
+		$this->eventLogger->start('route:run:call', 'Run callable route');
+		call_user_func($action, $parameters);
+		$this->eventLogger->end('route:run:call');
+	}
+
+	/**
 	 * Get the url generator
 	 *
 	 * @return \Symfony\Component\Routing\Generator\UrlGenerator
@@ -492,7 +512,7 @@ class Router implements IRouter {
 	 * @param string $file the route file location to include
 	 * @param string $appName
 	 */
-	private function requireRouteFile($file, $appName) {
+	protected function requireRouteFile(string $file, string $appName): void {
 		$this->setupRoutes(include $file, $appName);
 	}
 
diff --git a/lib/private/Server.php b/lib/private/Server.php
index bf07ee355ea..83eb95cd671 100644
--- a/lib/private/Server.php
+++ b/lib/private/Server.php
@@ -605,7 +605,7 @@ class Server extends ServerContainer implements IServerContainer {
 				$prefixClosure = function () use ($logQuery, $serverVersion): ?string {
 					if (!$logQuery) {
 						try {
-							$v = \OCP\Server::get(IAppConfig::class)->getAppInstalledVersions();
+							$v = \OCP\Server::get(IAppConfig::class)->getAppInstalledVersions(true);
 						} catch (\Doctrine\DBAL\Exception $e) {
 							// Database service probably unavailable
 							// Probably related to https://github.com/nextcloud/server/issues/37424
@@ -620,7 +620,7 @@ class Server extends ServerContainer implements IServerContainer {
 						];
 					}
 					$v['core'] = implode(',', $serverVersion->getVersion());
-					$version = implode(',', $v);
+					$version = implode(',', array_keys($v)) . implode(',', $v);
 					$instanceId = \OC_Util::getInstanceId();
 					$path = \OC::$SERVERROOT;
 					return md5($instanceId . '-' . $version . '-' . $path);
diff --git a/lib/private/TemplateLayout.php b/lib/private/TemplateLayout.php
index caffbfceefa..cfc387d2164 100644
--- a/lib/private/TemplateLayout.php
+++ b/lib/private/TemplateLayout.php
@@ -201,7 +201,7 @@ class TemplateLayout {
 
 		if ($this->config->getSystemValueBool('installed', false)) {
 			if (empty(self::$versionHash)) {
-				$v = $this->appManager->getAppInstalledVersions();
+				$v = $this->appManager->getAppInstalledVersions(true);
 				$v['core'] = implode('.', $this->serverVersion->getVersion());
 				self::$versionHash = substr(md5(implode(',', $v)), 0, 8);
 			}
diff --git a/lib/public/App/IAppManager.php b/lib/public/App/IAppManager.php
index 67ef2d796be..6eae870216f 100644
--- a/lib/public/App/IAppManager.php
+++ b/lib/public/App/IAppManager.php
@@ -57,7 +57,7 @@ interface IAppManager {
 	 * @return array<string, string>
 	 * @since 32.0.0
 	 */
-	public function getAppInstalledVersions(): array;
+	public function getAppInstalledVersions(bool $onlyEnabled = false): array;
 
 	/**
 	 * Returns the app icon or null if none is found
diff --git a/lib/public/IAppConfig.php b/lib/public/IAppConfig.php
index f4210793476..fcc528fe11f 100644
--- a/lib/public/IAppConfig.php
+++ b/lib/public/IAppConfig.php
@@ -514,5 +514,5 @@ interface IAppConfig {
 	 * @return array<string, string>
 	 * @since 32.0.0
 	 */
-	public function getAppInstalledVersions(): array;
+	public function getAppInstalledVersions(bool $onlyEnabled = false): array;
 }
diff --git a/lib/public/Route/IRoute.php b/lib/public/Route/IRoute.php
index fffd4b9c1a1..7dba6225aff 100644
--- a/lib/public/Route/IRoute.php
+++ b/lib/public/Route/IRoute.php
@@ -34,8 +34,9 @@ interface IRoute {
 	 * it is called directly
 	 *
 	 * @param string $file
-	 * @return void
+	 * @return $this
 	 * @since 7.0.0
+	 * @deprecated 32.0.0 Use a proper controller instead
 	 */
 	public function actionInclude($file);
 
@@ -70,6 +71,7 @@ interface IRoute {
 	 * This function is called with $class set to a callable or
 	 * to the class with $function
 	 * @since 7.0.0
+	 * @deprecated 32.0.0 Use a proper controller instead
 	 */
 	public function action($class, $function = null);
 
diff --git a/tests/lib/App/AppManagerTest.php b/tests/lib/App/AppManagerTest.php
index c8f7f2cb172..48d722761bc 100644
--- a/tests/lib/App/AppManagerTest.php
+++ b/tests/lib/App/AppManagerTest.php
@@ -71,6 +71,17 @@ class AppManagerTest extends TestCase {
 					return $values;
 				}
 			});
+		$config->expects($this->any())
+			->method('searchValues')
+			->willReturnCallback(function ($key, $lazy, $type) use (&$appConfig) {
+				$values = [];
+				foreach ($appConfig as $appid => $appData) {
+					if (isset($appData[$key])) {
+						$values[$appid] = $appData[$key];
+					}
+				}
+				return $values;
+			});
 
 		return $config;
 	}
diff --git a/tests/lib/AppConfigTest.php b/tests/lib/AppConfigTest.php
index faeaa4c4560..4acb83ece14 100644
--- a/tests/lib/AppConfigTest.php
+++ b/tests/lib/AppConfigTest.php
@@ -38,7 +38,7 @@ class AppConfigTest extends TestCase {
 	private static array $baseStruct =
 		[
 			'testapp' => [
-				'enabled' => ['enabled', 'true'],
+				'enabled' => ['enabled', 'yes'],
 				'installed_version' => ['installed_version', '1.2.3'],
 				'depends_on' => ['depends_on', 'someapp'],
 				'deletethis' => ['deletethis', 'deletethis'],
@@ -49,11 +49,12 @@ class AppConfigTest extends TestCase {
 				'otherkey' => ['otherkey', 'othervalue']
 			],
 			'123456' => [
-				'enabled' => ['enabled', 'true'],
+				'enabled' => ['enabled', 'yes'],
 				'key' => ['key', 'value']
 			],
 			'anotherapp' => [
-				'enabled' => ['enabled', 'false'],
+				'enabled' => ['enabled', 'no'],
+				'installed_version' => ['installed_version', '3.2.1'],
 				'key' => ['key', 'value']
 			],
 			'non-sensitive-app' => [
@@ -211,6 +212,19 @@ class AppConfigTest extends TestCase {
 		$this->assertEqualsCanonicalizing(array_keys(self::$baseStruct), $config->getApps());
 	}
 
+	public function testGetAppInstalledVersions(): void {
+		$config = $this->generateAppConfig(false);
+
+		$this->assertEquals(
+			['testapp' => '1.2.3', 'anotherapp' => '3.2.1'],
+			$config->getAppInstalledVersions(false)
+		);
+		$this->assertEquals(
+			['testapp' => '1.2.3'],
+			$config->getAppInstalledVersions(true)
+		);
+	}
+
 	/**
 	 * returns list of app and their keys
 	 *
@@ -410,7 +424,7 @@ class AppConfigTest extends TestCase {
 
 	public function testSearchValues(): void {
 		$config = $this->generateAppConfig();
-		$this->assertEqualsCanonicalizing(['testapp' => 'true', '123456' => 'true', 'anotherapp' => 'false'], $config->searchValues('enabled'));
+		$this->assertEqualsCanonicalizing(['testapp' => 'yes', '123456' => 'yes', 'anotherapp' => 'no'], $config->searchValues('enabled'));
 	}
 
 	public function testGetValueString(): void {
@@ -1322,7 +1336,7 @@ class AppConfigTest extends TestCase {
 		$config = $this->generateAppConfig();
 		$config->deleteKey('anotherapp', 'key');
 		$status = $config->statusCache();
-		$this->assertEqualsCanonicalizing(['enabled' => 'false'], $status['fastCache']['anotherapp']);
+		$this->assertEqualsCanonicalizing(['enabled' => 'no', 'installed_version' => '3.2.1'], $status['fastCache']['anotherapp']);
 	}
 
 	public function testDeleteKeyDatabase(): void {
diff --git a/tests/lib/AppTest.php b/tests/lib/AppTest.php
index b817d968910..cd33be9378d 100644
--- a/tests/lib/AppTest.php
+++ b/tests/lib/AppTest.php
@@ -485,7 +485,7 @@ class AppTest extends \Test\TestCase {
 		\OC_User::setUserId($user);
 
 		$this->setupAppConfigMock()->expects($this->once())
-			->method('getValues')
+			->method('searchValues')
 			->willReturn(
 				[
 					'app3' => 'yes',
@@ -495,7 +495,6 @@ class AppTest extends \Test\TestCase {
 					'appforgroup2' => '["group2"]',
 					'appforgroup12' => '["group2","group1"]',
 				]
-
 			);
 
 		$apps = \OC_App::getEnabledApps(false, $forceAll);
@@ -524,13 +523,12 @@ class AppTest extends \Test\TestCase {
 		\OC_User::setUserId(self::TEST_USER1);
 
 		$this->setupAppConfigMock()->expects($this->once())
-			->method('getValues')
+			->method('searchValues')
 			->willReturn(
 				[
 					'app3' => 'yes',
 					'app2' => 'no',
 				]
-
 			);
 
 		$apps = \OC_App::getEnabledApps();
@@ -550,7 +548,7 @@ class AppTest extends \Test\TestCase {
 	private function setupAppConfigMock() {
 		/** @var AppConfig|MockObject */
 		$appConfig = $this->getMockBuilder(AppConfig::class)
-			->onlyMethods(['getValues'])
+			->onlyMethods(['searchValues'])
 			->setConstructorArgs([\OCP\Server::get(IDBConnection::class)])
 			->disableOriginalConstructor()
 			->getMock();