drop ExApp scopes table, rewrite code of it (#285)

This is ongoing part of optimizations of reducing the number of database/cache requests. --------- Signed-off-by: Andrey Borysenko <[email protected]> Signed-off-by: Alexander Piskun <[email protected]> Co-authored-by: Alexander Piskun <[email protected]>
nextcloud · May 7, 2024 · 3d503b3 · 3d503b3
1 parent 2228f9e
commit 3d503b3
Show file tree

Hide file tree

Showing 10 changed files with 102 additions and 358 deletions.
diff --git a/lib/Command/ExApp/Register.php b/lib/Command/ExApp/Register.php
@@ -11,7 +11,6 @@
 use OCA\AppAPI\Service\AppAPIService;
 use OCA\AppAPI\Service\DaemonConfigService;
 use OCA\AppAPI\Service\ExAppApiScopeService;
-use OCA\AppAPI\Service\ExAppScopesService;
 use OCA\AppAPI\Service\ExAppService;
 
 use OCP\IConfig;
@@ -30,7 +29,6 @@ class Register extends Command {
 	public function __construct(
 		private readonly AppAPIService  	  $service,
 		private readonly DaemonConfigService  $daemonConfigService,
-		private readonly ExAppScopesService   $exAppScopesService,
 		private readonly ExAppApiScopeService $exAppApiScopeService,
 		private readonly DockerActions        $dockerActions,
 		private readonly ManualActions        $manualActions,
@@ -111,9 +109,8 @@ protected function execute(InputInterface $input, OutputInterface $output): int
 			return 2;
 		}
 
-		$forceScopes = (bool) $input->getOption('force-scopes');
-		$confirmRequiredScopes = $forceScopes;
-		if (!$forceScopes && $input->isInteractive()) {
+		$confirmRequiredScopes = (bool) $input->getOption('force-scopes');
+		if (!$confirmRequiredScopes && $input->isInteractive()) {
 			/** @var QuestionHelper $helper */
 			$helper = $this->getHelper('question');
 
@@ -147,14 +144,6 @@ protected function execute(InputInterface $input, OutputInterface $output): int
 			return 3;
 		}
 		if (count($appInfo['external-app']['scopes']) > 0) {
-			if (!$this->exAppScopesService->registerExAppScopes($exApp, $this->exAppApiScopeService->mapScopeGroupsToNumbers($appInfo['external-app']['scopes']))) {
-				$this->logger->error(sprintf('Error while registering API scopes for %s.', $appId));
-				if ($outputConsole) {
-					$output->writeln(sprintf('Error while registering API scopes for %s.', $appId));
-				}
-				$this->_unregisterExApp($appId, $isTestDeployMode);
-				return 1;
-			}
 			$this->logger->info(
 				sprintf('ExApp %s scope groups successfully set: %s', $exApp->getAppid(), implode(', ', $appInfo['external-app']['scopes']))
 			);

diff --git a/lib/Command/ExApp/Scopes/ListScopes.php b/lib/Command/ExApp/Scopes/ListScopes.php
@@ -4,9 +4,7 @@
 
 namespace OCA\AppAPI\Command\ExApp\Scopes;
 
-use OCA\AppAPI\Db\ExAppScope;
 use OCA\AppAPI\Service\ExAppApiScopeService;
-use OCA\AppAPI\Service\ExAppScopesService;
 
 use OCA\AppAPI\Service\ExAppService;
 use Symfony\Component\Console\Command\Command;
@@ -18,7 +16,6 @@ class ListScopes extends Command {
 
 	public function __construct(
 		private readonly ExAppService         $service,
-		private readonly ExAppScopesService   $exAppScopeService,
 		private readonly ExAppApiScopeService $exAppApiScopeService,
 	) {
 		parent::__construct();
@@ -39,16 +36,14 @@ protected function execute(InputInterface $input, OutputInterface $output): int
 			return 2;
 		}
 
-		$scopes = $this->exAppScopeService->getExAppScopes($exApp);
+		$scopes = $exApp->getApiScopes();
 		if (empty($scopes)) {
 			$output->writeln(sprintf('No scopes granted for ExApp %s', $appId));
 			return 0;
 		}
 
 		$output->writeln(sprintf('ExApp %s scopes:', $exApp->getAppid()));
-		$mappedScopes = array_unique($this->exAppApiScopeService->mapScopeGroupsToNames(array_map(function (ExAppScope $scope) {
-			return intval($scope->getScopeGroup());
-		}, $scopes)));
+		$mappedScopes = array_unique($this->exAppApiScopeService->mapScopeGroupsToNames($scopes));
 		$output->writeln(join(', ', $mappedScopes));
 		return 0;
 	}

diff --git a/lib/Command/ExApp/Update.php b/lib/Command/ExApp/Update.php
@@ -4,15 +4,13 @@
 
 namespace OCA\AppAPI\Command\ExApp;
 
-use OCA\AppAPI\Db\ExAppScope;
 use OCA\AppAPI\DeployActions\DockerActions;
 use OCA\AppAPI\DeployActions\ManualActions;
 use OCA\AppAPI\Fetcher\ExAppArchiveFetcher;
 use OCA\AppAPI\Fetcher\ExAppFetcher;
 use OCA\AppAPI\Service\AppAPIService;
 use OCA\AppAPI\Service\DaemonConfigService;
 use OCA\AppAPI\Service\ExAppApiScopeService;
-use OCA\AppAPI\Service\ExAppScopesService;
 
 use OCA\AppAPI\Service\ExAppService;
 use Psr\Log\LoggerInterface;
@@ -29,7 +27,6 @@ class Update extends Command {
 	public function __construct(
 		private readonly AppAPIService  	  $service,
 		private readonly ExAppService         $exAppService,
-		private readonly ExAppScopesService   $exAppScopeService,
 		private readonly ExAppApiScopeService $exAppApiScopeService,
 		private readonly DaemonConfigService  $daemonConfigService,
 		private readonly DockerActions        $dockerActions,
@@ -141,6 +138,30 @@ private function updateExApp(InputInterface $input, OutputInterface $output, str
 			return 0;
 		}
 
+		// Default scopes approval process (compare new ExApp scopes)
+		$currentExAppScopes = $exApp->getApiScopes();
+		// Prepare for prompt of newly requested ExApp scopes
+		$requiredScopes = array_values(array_diff($this->exAppApiScopeService->mapScopeGroupsToNumbers($appInfo['external-app']['scopes']), $currentExAppScopes));
+
+		$confirmScopes = (bool) $input->getOption('force-scopes');
+		if (!$confirmScopes && $input->isInteractive()) {
+			/** @var QuestionHelper $helper */
+			$helper = $this->getHelper('question');
+
+			if (count($requiredScopes) > 0) {
+				$output->writeln(sprintf('ExApp %s requested scopes: %s', $appId, implode(', ',
+					$this->exAppApiScopeService->mapScopeGroupsToNames($requiredScopes))));
+				$question = new ConfirmationQuestion('Do you want to approve it? [y/N] ', false);
+				$confirmScopes = $helper->ask($input, $output, $question);
+			} else {
+				$confirmScopes = true;
+			}
+		}
+		if (!$confirmScopes && count($requiredScopes) > 0) {
+			$output->writeln(sprintf('ExApp %s required scopes not approved. Failed to finish ExApp update.', $appId));
+			return 1;
+		}
+
 		$status = $exApp->getStatus();
 		$status['type'] = 'update';
 		$status['error'] = '';
@@ -239,46 +260,6 @@ private function updateExApp(InputInterface $input, OutputInterface $output, str
 			$output->writeln(sprintf('ExApp %s update successfully deployed.', $appId));
 		}
 
-		// Default scopes approval process (compare new ExApp scopes)
-		$currentExAppScopes = array_map(function (ExAppScope $exAppScope) {
-			return $exAppScope->getScopeGroup();
-		}, $this->exAppScopeService->getExAppScopes($exApp));
-		// Prepare for prompt of newly requested ExApp scopes
-		$requiredScopes = array_values(array_diff($this->exAppApiScopeService->mapScopeGroupsToNumbers($appInfo['external-app']['scopes']), $currentExAppScopes));
-
-		$forceScopes = (bool) $input->getOption('force-scopes');
-		$confirmScopes = $forceScopes;
-
-		if (!$forceScopes && $input->isInteractive()) {
-			/** @var QuestionHelper $helper */
-			$helper = $this->getHelper('question');
-
-			if (count($requiredScopes) > 0) {
-				$output->writeln(sprintf('ExApp %s requested scopes: %s', $appId, implode(', ',
-					$this->exAppApiScopeService->mapScopeGroupsToNames($requiredScopes))));
-				$question = new ConfirmationQuestion('Do you want to approve it? [y/N] ', false);
-				$confirmScopes = $helper->ask($input, $output, $question);
-			} else {
-				$confirmScopes = true;
-			}
-		}
-
-		if (!$confirmScopes && count($requiredScopes) > 0) {
-			$output->writeln(sprintf('ExApp %s required scopes not approved. Failed to finish ExApp update.', $appId));
-			return 1;
-		}
-
-		if (!$this->exAppScopeService->registerExAppScopes(
-			$exApp, $this->exAppApiScopeService->mapScopeGroupsToNumbers($appInfo['external-app']['scopes']))
-		) {
-			$this->logger->error(sprintf('Failed to update ExApp %s scopes.', $appId));
-			if ($outputConsole) {
-				$output->writeln(sprintf('Failed to update ExApp %s scopes.', $appId));
-			}
-			$this->exAppService->setStatusError($exApp, 'Failed to update scopes');
-			return 1;
-		}
-
 		$this->service->dispatchExAppInitInternal($exApp);
 		if ($input->getOption('wait-finish')) {
 			$error = $this->exAppService->waitInitStepFinish($appId);

diff --git a/lib/Controller/ExAppsPageController.php b/lib/Controller/ExAppsPageController.php
@@ -12,13 +12,11 @@
 use OC\App\Platform;
 use OC_App;
 use OCA\AppAPI\AppInfo\Application;
-use OCA\AppAPI\Db\ExAppScope;
 use OCA\AppAPI\DeployActions\DockerActions;
 use OCA\AppAPI\Fetcher\ExAppFetcher;
 use OCA\AppAPI\Service\AppAPIService;
 use OCA\AppAPI\Service\DaemonConfigService;
 use OCA\AppAPI\Service\ExAppApiScopeService;
-use OCA\AppAPI\Service\ExAppScopesService;
 use OCA\AppAPI\Service\ExAppService;
 use OCA\AppAPI\Service\ExAppUsersService;
 use OCP\App\IAppManager;
@@ -45,7 +43,6 @@ class ExAppsPageController extends Controller {
 	private IConfig $config;
 	private AppAPIService $service;
 	private DaemonConfigService $daemonConfigService;
-	private ExAppScopesService $exAppScopeService;
 	private DockerActions $dockerActions;
 	private CategoryFetcher $categoryFetcher;
 	private IFactory $l10nFactory;
@@ -62,7 +59,6 @@ public function __construct(
 		IInitialState $initialStateService,
 		AppAPIService $service,
 		DaemonConfigService $daemonConfigService,
-		ExAppScopesService $exAppScopeService,
 		ExAppApiScopeService $exAppApiScopeService,
 		ExAppUsersService $exAppUsersService,
 		DockerActions $dockerActions,
@@ -80,7 +76,6 @@ public function __construct(
 		$this->config = $config;
 		$this->service = $service;
 		$this->daemonConfigService = $daemonConfigService;
-		$this->exAppScopeService = $exAppScopeService;
 		$this->exAppApiScopeService = $exAppApiScopeService;
 		$this->dockerActions = $dockerActions;
 		$this->categoryFetcher = $categoryFetcher;
@@ -205,9 +200,7 @@ private function getAppsForCategory(string $requestedCategory = ''): array {
 			$daemon = null;
 
 			if ($exApp !== null) {
-				$scopes = $this->exAppApiScopeService->mapScopeGroupsToNames(array_map(function (ExAppScope $exAppScope) {
-					return $exAppScope->getScopeGroup();
-				}, $this->exAppScopeService->getExAppScopes($exApp)));
+				$scopes = $this->exAppApiScopeService->mapScopeGroupsToNames($exApp->getApiScopes());
 				$daemon = $this->daemonConfigService->getDaemonConfigByName($exApp->getDaemonConfigName());
 			}
 
@@ -342,9 +335,7 @@ private function buildLocalAppsList(array $apps, array $exApps): array {
 			if (!in_array($app['id'], $registeredAppsIds)) {
 				$exApp = $this->exAppService->getExApp($app['id']);
 				$daemon = $this->daemonConfigService->getDaemonConfigByName($exApp->getDaemonConfigName());
-				$scopes = $this->exAppApiScopeService->mapScopeGroupsToNames(array_map(function (ExAppScope $exAppScope) {
-					return $exAppScope->getScopeGroup();
-				}, $this->exAppScopeService->getExAppScopes($exApp)));
+				$scopes = $this->exAppApiScopeService->mapScopeGroupsToNames($exApp->getApiScopes());
 
 				$formattedLocalApps[] = [
 					'id' => $app['id'],

diff --git a/lib/Db/ExAppScope.php b/lib/Db/ExAppScope.php
diff --git a/lib/Db/ExAppScopeMapper.php b/lib/Db/ExAppScopeMapper.php
diff --git a/lib/Migration/Version2207Date20240502145029.php b/lib/Migration/Version2207Date20240502145029.php
@@ -0,0 +1,30 @@
+<?php
+
+declare(strict_types=1);
+
+namespace OCA\AppAPI\Migration;
+
+use Closure;
+use OCP\DB\ISchemaWrapper;
+use OCP\Migration\IOutput;
+use OCP\Migration\SimpleMigrationStep;
+
+class Version2207Date20240502145029 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();
+
+		if ($schema->hasTable('ex_apps_scopes')) {
+			$schema->dropTable('ex_apps_scopes');
+		}
+
+		return $schema;
+	}
+}