From 1d0cd7328b10875f5b6bb0fa23153f338df18461 Mon Sep 17 00:00:00 2001
From: Hypolite Petovan <hypolite@mrpetovan.com>
Date: Mon, 27 Jul 2020 01:57:44 -0400
Subject: [PATCH] Add dispatch data caching in App\Router

- Add new cache key "routerDispatchData"
- Update Dice dependencies since Router constructor signature changed
---
 src/App/Router.php             | 78 ++++++++++++++++++++++++++++++----
 src/Core/Hook.php              | 12 ++++++
 static/dependencies.config.php |  7 ++-
 3 files changed, 85 insertions(+), 12 deletions(-)

diff --git a/src/App/Router.php b/src/App/Router.php
index 8094e3b46d..dfe890fb96 100644
--- a/src/App/Router.php
+++ b/src/App/Router.php
@@ -26,6 +26,8 @@ use FastRoute\DataGenerator\GroupCountBased;
 use FastRoute\Dispatcher;
 use FastRoute\RouteCollector;
 use FastRoute\RouteParser\Std;
+use Friendica\Core\Cache\Duration;
+use Friendica\Core\Cache\ICache;
 use Friendica\Core\Hook;
 use Friendica\Core\L10n;
 use Friendica\Network\HTTPException;
@@ -66,14 +68,24 @@ class Router
 	/** @var L10n */
 	private $l10n;
 
+	/** @var ICache */
+	private $cache;
+
+	/** @var string */
+	private $baseRoutesFilepath;
+
 	/**
-	 * @param array $server The $_SERVER variable
-	 * @param L10n  $l10n
-	 * @param RouteCollector|null $routeCollector Optional the loaded Route collector
+	 * @param array               $server             The $_SERVER variable
+	 * @param string              $baseRoutesFilepath The path to a base routes file to leverage cache, can be empty
+	 * @param L10n                $l10n
+	 * @param ICache              $cache
+	 * @param RouteCollector|null $routeCollector
 	 */
-	public function __construct(array $server, L10n $l10n, RouteCollector $routeCollector = null)
+	public function __construct(array $server, string $baseRoutesFilepath, L10n $l10n, ICache $cache, RouteCollector $routeCollector = null)
 	{
+		$this->baseRoutesFilepath = $baseRoutesFilepath;
 		$this->l10n = $l10n;
+		$this->cache = $cache;
 
 		$httpMethod = $server['REQUEST_METHOD'] ?? self::GET;
 		$this->httpMethod = in_array($httpMethod, self::ALLOWED_METHODS) ? $httpMethod : self::GET;
@@ -84,6 +96,9 @@ class Router
 	}
 
 	/**
+	 * This will be called either automatically if a base routes file path was submitted,
+	 * or can be called manually with a custom route array.
+	 *
 	 * @param array $routes The routes to add to the Router
 	 *
 	 * @return self The router instance with the loaded routes
@@ -100,6 +115,9 @@ class Router
 
 		$this->routeCollector = $routeCollector;
 
+		// Add routes from addons
+		Hook::callAll('route_collection', $this->routeCollector);
+
 		return $this;
 	}
 
@@ -191,12 +209,9 @@ class Router
 	 */
 	public function getModuleClass($cmd)
 	{
-		// Add routes from addons
-		Hook::callAll('route_collection', $this->routeCollector);
-
 		$cmd = '/' . ltrim($cmd, '/');
 
-		$dispatcher = new Dispatcher\GroupCountBased($this->routeCollector->getData());
+		$dispatcher = new Dispatcher\GroupCountBased($this->getCachedDispatchData());
 
 		$moduleClass = null;
 		$this->parameters = [];
@@ -223,4 +238,51 @@ class Router
 	{
 		return $this->parameters;
 	}
+
+	/**
+	 * If a base routes file path has been provided, we can load routes from it if the cache misses.
+	 *
+	 * @return array
+	 * @throws HTTPException\InternalServerErrorException
+	 */
+	private function getDispatchData()
+	{
+		$dispatchData = [];
+
+		if ($this->baseRoutesFilepath && file_exists($this->baseRoutesFilepath)) {
+			$dispatchData = require $this->baseRoutesFilepath;
+			if (!is_array($dispatchData)) {
+				throw new HTTPException\InternalServerErrorException('Invalid base routes file');
+			}
+		}
+
+		$this->loadRoutes($dispatchData);
+
+		return $this->routeCollector->getData();
+	}
+
+	/**
+	 * We cache the dispatch data for speed, as computing the current routes (version 2020.09)
+	 * takes about 850ms for each requests.
+	 *
+	 * The cached "routerDispatchData" lasts for a day, and must be cleared manually when there
+	 * is any changes in the enabled addons list.
+	 *
+	 * @return array|mixed
+	 * @throws HTTPException\InternalServerErrorException
+	 */
+	private function getCachedDispatchData()
+	{
+		$routerDispatchData = $this->cache->get('routerDispatchData');
+
+		if ($routerDispatchData) {
+			return $routerDispatchData;
+		}
+
+		$routerDispatchData = $this->getDispatchData();
+
+		$this->cache->set('routerDispatchData', $routerDispatchData, Duration::DAY);
+
+		return $routerDispatchData;
+	}
 }
diff --git a/src/Core/Hook.php b/src/Core/Hook.php
index 851dd60ed9..d7b1f737a0 100644
--- a/src/Core/Hook.php
+++ b/src/Core/Hook.php
@@ -247,6 +247,8 @@ class Hook
 	/**
 	 * Deletes one or more hook records
 	 *
+	 * We have to clear the cached routerDispatchData because addons can provide routes
+	 *
 	 * @param array $condition
 	 * @param array $options
 	 * @return bool
@@ -256,12 +258,18 @@ class Hook
 	{
 		$result = DBA::delete('hook', $condition, $options);
 
+		if ($result) {
+			DI::cache()->delete('routerDispatchData');
+		}
+
 		return $result;
 	}
 
 	/**
 	 * Inserts a hook record
 	 *
+	 * We have to clear the cached routerDispatchData because addons can provide routes
+	 *
 	 * @param array $condition
 	 * @return bool
 	 * @throws \Exception
@@ -270,6 +278,10 @@ class Hook
 	{
 		$result = DBA::insert('hook', $condition);
 
+		if ($result) {
+			DI::cache()->delete('routerDispatchData');
+		}
+
 		return $result;
 	}
 }
diff --git a/static/dependencies.config.php b/static/dependencies.config.php
index fe8a8caee0..3df54b79e6 100644
--- a/static/dependencies.config.php
+++ b/static/dependencies.config.php
@@ -191,10 +191,9 @@ return [
 	],
 	App\Router::class => [
 		'constructParams' => [
-			$_SERVER, null
-		],
-		'call' => [
-			['loadRoutes', [include __DIR__ . '/routes.config.php'], Dice::CHAIN_CALL],
+			$_SERVER,
+			__DIR__ . '/routes.config.php',
+			null
 		],
 	],
 	L10n::class => [