performance: store API scopes as a list in the main table (#254)

Signed-off-by: Alexander Piskun <[email protected]>
nextcloud · Mar 27, 2024 · f7eefb8 · f7eefb8
1 parent 57b34d0
commit f7eefb8
Show file tree

Hide file tree

Showing 7 changed files with 57 additions and 9 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -5,7 +5,7 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](http://keepachangelog.com/)
 and this project adheres to [Semantic Versioning](http://semver.org/).
 
-## [2.3.2 - 2024-03-xx]
+## [2.3.2 - 2024-03-26]
 
 ### Fixed
 

diff --git a/lib/Command/ExApp/Register.php b/lib/Command/ExApp/Register.php
@@ -134,6 +134,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int
 		$appInfo['port'] = $appInfo['port'] ?? $this->exAppService->getExAppFreePort();
 		$appInfo['secret'] = $appInfo['secret'] ?? $this->random->generate(128);
 		$appInfo['daemon_config_name'] = $appInfo['daemon_config_name'] ?? $daemonConfigName;
+		$appInfo['api_scopes'] = $this->exAppApiScopeService->mapScopeNamesToNumbers($appInfo['external-app']['scopes']);
 		$exApp = $this->exAppService->registerExApp($appInfo);
 		if (!$exApp) {
 			$this->logger->error(sprintf('Error during registering ExApp %s.', $appId));
@@ -143,8 +144,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int
 			return 3;
 		}
 		if (count($appInfo['external-app']['scopes']) > 0) {
-			if (!$this->exAppScopesService->registerExAppScopes(
-				$exApp, $this->exAppApiScopeService->mapScopeNamesToNumbers($appInfo['external-app']['scopes']))
+			if (!$this->exAppScopesService->registerExAppScopes($exApp, $appInfo['api_scopes'])
 			) {
 				$this->logger->error(sprintf('Error while registering API scopes for %s.', $appId));
 				if ($outputConsole) {

diff --git a/lib/Command/ExApp/Update.php b/lib/Command/ExApp/Update.php
@@ -136,6 +136,7 @@ protected function execute(InputInterface $input, OutputInterface $output): int
 			}
 		}
 
+		$appInfo['api_scopes'] = $this->exAppApiScopeService->mapScopeNamesToNumbers($appInfo['external-app']['scopes']);
 		if (!$this->exAppService->updateExAppInfo($exApp, $appInfo)) {
 			$this->logger->error(sprintf('Failed to update ExApp %s info', $appId));
 			if ($outputConsole) {

diff --git a/lib/Db/ExApp.php b/lib/Db/ExApp.php
@@ -25,6 +25,7 @@
  * @method int getCreatedTime()
  * @method int getLastCheckTime()
  * @method int getIsSystem()
+ * @method array getApiScopes()
  * @method array getDeployConfig()
  * @method string getAcceptsDeployId()
  * @method void setAppid(string $appid)
@@ -40,6 +41,7 @@
  * @method void setCreatedTime(int $createdTime)
  * @method void setLastCheckTime(int $lastCheckTime)
  * @method void setIsSystem(int $system)
+ * @method void setApiScopes(array $apiScopes)
  * @method void setDeployConfig(array $deployConfig)
  * @method void setAcceptsDeployId(string $acceptsDeployId)
  */
@@ -57,6 +59,7 @@ class ExApp extends Entity implements JsonSerializable {
 	protected $createdTime;
 	protected $lastCheckTime;
 	protected $isSystem;
+	protected $apiScopes;
 	protected $deployConfig;
 	protected $acceptsDeployId;
 
@@ -77,6 +80,7 @@ public function __construct(array $params = []) {
 		$this->addType('createdTime', 'int');
 		$this->addType('lastCheckTime', 'int');
 		$this->addType('isSystem', 'int');
+		$this->addType('apiScopes', 'json');
 		$this->addType('deployConfig', 'json');
 		$this->addType('acceptsDeployId', 'string');
 
@@ -122,6 +126,11 @@ public function __construct(array $params = []) {
 		if (isset($params['is_system'])) {
 			$this->setIsSystem($params['is_system']);
 		}
+		if (isset($params['api_scopes'])) {
+			$this->setApiScopes($params['api_scopes']);
+		} else {
+			$this->setApiScopes([]);
+		}
 		if (isset($params['deploy_config'])) {
 			$this->setDeployConfig($params['deploy_config']);
 		}
@@ -146,6 +155,7 @@ public function jsonSerialize(): array {
 			'created_time' => $this->getCreatedTime(),
 			'last_check_time' => $this->getLastCheckTime(),
 			'is_system' => $this->getIsSystem(),
+			'api_scopes' => $this->getApiScopes(),
 			'deploy_config' => $this->getDeployConfig(),
 			'accepts_deploy_id' => $this->getAcceptsDeployId(),
 		];

diff --git a/lib/Db/ExAppMapper.php b/lib/Db/ExAppMapper.php
@@ -116,6 +116,8 @@ public function updateExApp(ExApp $exApp, array $fields): int {
 				$qb = $qb->set('last_check_time', $qb->createNamedParameter($exApp->getLastCheckTime(), IQueryBuilder::PARAM_INT));
 			} elseif ($field === 'is_system') {
 				$qb = $qb->set('is_system', $qb->createNamedParameter($exApp->getIsSystem(), IQueryBuilder::PARAM_INT));
+			} elseif ($field === 'api_scopes') {
+				$qb = $qb->set('api_scopes', $qb->createNamedParameter($exApp->getApiScopes(), IQueryBuilder::PARAM_JSON));
 			}
 		}
 		return $qb->where($qb->expr()->eq('appid', $qb->createNamedParameter($exApp->getAppid())))->executeStatement();

diff --git a/lib/Migration/Version2203Date20240325124149.php b/lib/Migration/Version2203Date20240325124149.php
@@ -0,0 +1,33 @@
+<?php
+
+declare(strict_types=1);
+
+namespace OCA\AppAPI\Migration;
+
+use Closure;
+use OCP\DB\ISchemaWrapper;
+use OCP\DB\Types;
+use OCP\Migration\IOutput;
+use OCP\Migration\SimpleMigrationStep;
+
+class Version2203Date20240325124149 extends SimpleMigrationStep {
+	/**
+	 * @param IOutput $output
+	 * @param Closure $schemaClosure The `\Closure` returns a `ISchemaWrapper`
+	 * @param array $options
+	 *
+	 * @return null|ISchemaWrapper
+	 */
+	public function changeSchema(IOutput $output, Closure $schemaClosure, array $options) {
+		/** @var ISchemaWrapper $schema */
+		$schema = $schemaClosure();
+
+		$table = $schema->getTable('ex_apps');
+
+		$table->addColumn('api_scopes', Types::JSON, [
+			'notnull' => false,
+		]);
+
+		return $schema;
+	}
+}
diff --git a/lib/Service/ExAppService.php b/lib/Service/ExAppService.php
@@ -78,6 +78,7 @@ public function registerExApp(array $appInfo): ?ExApp {
 			'created_time' => time(),
 			'last_check_time' => time(),
 			'is_system' => (int)filter_var($appInfo['external-app']['system'], FILTER_VALIDATE_BOOLEAN),
+			'api_scopes' => $appInfo['api_scopes'],
 		]);
 		try {
 			$this->exAppMapper->insert($exApp);
@@ -195,13 +196,14 @@ public function updateExAppInfo(ExApp $exApp, array $appInfo): bool {
 		$exApp->setVersion($appInfo['version']);
 		$exApp->setName($appInfo['name']);
 		$exApp->setIsSystem((int)filter_var($appInfo['external-app']['system'], FILTER_VALIDATE_BOOLEAN));
-		if (!$this->updateExApp($exApp, ['version', 'name', 'is_system'])) {
+		$exApp->setApiScopes($appInfo['api_scopes']);
+		if (!$this->updateExApp($exApp, ['version', 'name', 'is_system', 'api_scopes'])) {
 			return false;
 		}
 		return true;
 	}
 
-	public function updateExApp(ExApp $exApp, array $fields = ['version', 'name', 'port', 'status', 'enabled', 'last_check_time', 'is_system']): bool {
+	public function updateExApp(ExApp $exApp, array $fields = ['version', 'name', 'port', 'status', 'enabled', 'last_check_time', 'is_system', 'api_scopes']): bool {
 		try {
 			$this->exAppMapper->updateExApp($exApp, $fields);
 			$this->cache->remove('/ex_apps');
@@ -242,16 +244,16 @@ public function getAppInfo(string $appId, ?string $infoXml, ?string $jsonInfo):
 			# fill 'id' if it is missing(this field was called `appid` in previous versions in json)
 			$appInfo['id'] = $appInfo['id'] ?? $appId;
 			# during manual install JSON can have all values at root level
-			foreach (['docker-install', 'scopes', 'is_system', 'translations_folder'] as $key) {
+			foreach (['docker-install', 'scopes', 'system', 'system_app', 'translations_folder'] as $key) {
 				if (isset($appInfo[$key])) {
 					$appInfo['external-app'][$key] = $appInfo[$key];
 					unset($appInfo[$key]);
 				}
 			}
 			# TO-DO: remove this in AppAPI 2.4.0
-			if (isset($appInfo['system_app'])) {
-				$appInfo['external-app']['system'] = $appInfo['system_app'];
-				unset($appInfo['system_app']);
+			if (isset($appInfo['external-app']['system_app'])) {
+				$appInfo['external-app']['system'] = $appInfo['external-app']['system_app'];
+				unset($appInfo['external-app']['system_app']);
 			}
 		} else {
 			if ($infoXml !== null) {