occ app_api:app:unregister rework (#127)

Fixes #121 - small actions clean up - added basic but very useful tests for "occ app_api:app:unregister" Changes in behaviour: - container of app is always removed - volume with data is removed by default, but optionally can be kept - app_api does not try to disable ExApp if it is already disabled - "--silent" options now means "max silence as possible" - added "--force" option to ignore errors if any. --------- Signed-off-by: Alexander Piskun <[email protected]>
nextcloud · Nov 26, 2023 · 9ce6fc3 · 9ce6fc3
1 parent db5405a
commit 9ce6fc3
Show file tree

Hide file tree

Showing 9 changed files with 199 additions and 81 deletions.
diff --git a/.github/workflows/tests-deploy.yml b/.github/workflows/tests-deploy.yml
@@ -50,13 +50,6 @@ jobs:
           repository: nextcloud/server
           ref: ${{ matrix.server-version }}
 
-      - name: Checkout Notifications
-        uses: actions/checkout@v3
-        with:
-          repository: nextcloud/notifications
-          ref: ${{ matrix.server-version }}
-          path: apps/notifications
-
       - name: Checkout AppAPI
         uses: actions/checkout@c85c95e3d7251135ab7dc9ce3241c5835cc595a9 # v3.5.3
         with:
@@ -69,6 +62,8 @@ jobs:
           extensions: bz2, ctype, curl, dom, fileinfo, gd, iconv, intl, json, libxml, mbstring, openssl, pcntl, posix, session, simplexml, xmlreader, xmlwriter, zip, zlib, pgsql, pdo_pgsql
           coverage: none
           ini-file: development
+          ini-values:
+            apc.enabled=on, apc.enable_cli=on, disable_functions=
         env:
           GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
 
@@ -93,7 +88,6 @@ jobs:
             --admin-user admin --admin-pass admin
           ./occ config:system:set loglevel --value=0 --type=integer
           ./occ config:system:set debug --value=true --type=boolean
-          ./occ app:enable notifications
           ./occ app:enable --force ${{ env.APP_NAME }}
 
       - name: Test deploy
@@ -106,8 +100,6 @@ jobs:
             --info-xml https://raw.githubusercontent.com/cloud-py-api/nc_py_api/main/examples/as_app/skeleton/appinfo/info.xml
           ./occ app_api:app:enable skeleton
           ./occ app_api:app:disable skeleton
-          ./occ app_api:app:unregister skeleton --silent
-          ./occ app_api:daemon:unregister docker_local_sock
 
       - name: Check logs
         run: |
@@ -120,6 +112,14 @@ jobs:
           docker inspect nc_app_skeleton | json_pp > container.json
           docker logs nc_app_skeleton > container.log 2>&1
 
+      - name: Unregister Skeleton & Daemon
+        run: |
+          ./occ app_api:app:unregister skeleton
+          ./occ app_api:daemon:unregister docker_local_sock
+
+      - name: Test OCC commands(docker)
+        run: python3 apps/${{ env.APP_NAME }}/tests/test_occ_commands_docker.py
+
       - name: Upload Container info
         if: always()
         uses: actions/upload-artifact@v3
@@ -176,8 +176,6 @@ jobs:
             --info-xml https://raw.githubusercontent.com/cloud-py-api/nc_py_api/main/examples/as_app/skeleton/appinfo/info.xml
           docker exec nextcloud sudo -u www-data php occ app_api:app:enable skeleton
           docker exec nextcloud sudo -u www-data php occ app_api:app:disable skeleton
-          docker exec nextcloud sudo -u www-data php occ app_api:app:unregister skeleton --silent
-          docker exec nextcloud sudo -u www-data php occ app_api:daemon:unregister docker_local_sock
 
       - name: Copy NC log to host
         run: docker cp nextcloud:/var/www/html/data/nextcloud.log nextcloud.log
@@ -193,6 +191,11 @@ jobs:
           docker inspect nc_app_skeleton | json_pp > container.json
           docker logs nc_app_skeleton > container.log 2>&1
 
+      - name: Unregister Skeleton & Daemon
+        run: |
+          docker exec nextcloud sudo -u www-data php occ app_api:app:unregister skeleton
+          docker exec nextcloud sudo -u www-data php occ app_api:daemon:unregister docker_local_sock
+
       - name: Upload Container info
         if: always()
         uses: actions/upload-artifact@v3
@@ -254,8 +257,6 @@ jobs:
             --info-xml https://raw.githubusercontent.com/cloud-py-api/nc_py_api/main/examples/as_app/skeleton/appinfo/info.xml
           docker exec nextcloud sudo -u www-data php occ app_api:app:enable skeleton
           docker exec nextcloud sudo -u www-data php occ app_api:app:disable skeleton
-          docker exec nextcloud sudo -u www-data php occ app_api:app:unregister skeleton --silent
-          docker exec nextcloud sudo -u www-data php occ app_api:daemon:unregister docker_by_port
 
       - name: Copy NC log to host
         run: docker cp nextcloud:/var/www/html/data/nextcloud.log nextcloud.log
@@ -271,6 +272,11 @@ jobs:
           docker inspect nc_app_skeleton | json_pp > container.json
           docker logs nc_app_skeleton > container.log 2>&1
 
+      - name: Unregister Skeleton & Daemon
+        run: |
+          docker exec nextcloud sudo -u www-data php occ app_api:app:unregister skeleton
+          docker exec nextcloud sudo -u www-data php occ app_api:daemon:unregister docker_by_port
+
       - name: Upload Container info
         if: always()
         uses: actions/upload-artifact@v3
@@ -331,8 +337,6 @@ jobs:
             --info-xml https://raw.githubusercontent.com/cloud-py-api/nc_py_api/main/examples/as_app/skeleton/appinfo/info.xml
           docker exec nextcloud sudo -u www-data php occ app_api:app:enable skeleton
           docker exec nextcloud sudo -u www-data php occ app_api:app:disable skeleton
-          docker exec nextcloud sudo -u www-data php occ app_api:app:unregister skeleton --silent
-          docker exec nextcloud sudo -u www-data php occ app_api:daemon:unregister docker_by_port
 
       - name: Copy NC log to host
         run: docker cp nextcloud:/var/www/html/data/nextcloud.log nextcloud.log
@@ -348,6 +352,11 @@ jobs:
           docker inspect nc_app_skeleton | json_pp > container.json
           docker logs nc_app_skeleton > container.log 2>&1
 
+      - name: Unregister Skeleton & Daemon
+        run: |
+          docker exec nextcloud sudo -u www-data php occ app_api:app:unregister skeleton
+          docker exec nextcloud sudo -u www-data php occ app_api:daemon:unregister docker_by_port
+
       - name: Upload Container info
         if: always()
         uses: actions/upload-artifact@v3
@@ -473,8 +482,6 @@ jobs:
             --info-xml https://raw.githubusercontent.com/cloud-py-api/nc_py_api/main/examples/as_app/skeleton/appinfo/info.xml
           ./occ app_api:app:enable skeleton
           ./occ app_api:app:disable skeleton
-          ./occ app_api:app:unregister skeleton --silent
-          ./occ app_api:daemon:unregister docker_local_sock
 
       - name: Check logs
         run: |
@@ -487,6 +494,14 @@ jobs:
           docker inspect nc_app_skeleton | json_pp > container.json
           docker logs nc_app_skeleton > container.log 2>&1
 
+      - name: Unregister Skeleton & Daemon
+        run: |
+          ./occ app_api:app:unregister skeleton
+          ./occ app_api:daemon:unregister docker_local_sock
+
+      - name: Test OCC commands(docker)
+        run: python3 apps/${{ env.APP_NAME }}/tests/test_occ_commands_docker.py
+
       - name: Check redis keys
         run: |
           docker exec redis redis-cli keys '*app_api*' || error

diff --git a/.github/workflows/tests-special.yml b/.github/workflows/tests-special.yml
@@ -13,16 +13,17 @@ concurrency:
   group: tests-special-${{ github.head_ref || github.run_id }}
   cancel-in-progress: true
 
+env:
+  NEXTCLOUD_URL: "http://localhost:8080/"
+  APP_ID: "nc_py_api"
+  APP_PORT: 9009
+  APP_VERSION: "1.0.0"
+  APP_SECRET: "tC6vkwPhcppjMykD1r0n9NlI95uJMBYjs5blpIcA1PAdoPDmc5qoAjaBAkyocZ6E"
+
 jobs:
   app-version-higher:
     runs-on: ubuntu-22.04
     name: ExApp version higher
-    env:
-      NEXTCLOUD_URL: "http://localhost:8080/"
-      APP_ID: "nc_py_api"
-      APP_PORT: 9009
-      APP_VERSION: "1.0.0"
-      APP_SECRET: "tC6vkwPhcppjMykD1r0n9NlI95uJMBYjs5blpIcA1PAdoPDmc5qoAjaBAkyocZ6E"
 
     services:
       postgres:

diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml
@@ -13,6 +13,14 @@ concurrency:
   group: tests-${{ github.head_ref || github.run_id }}
   cancel-in-progress: true
 
+env:
+  NEXTCLOUD_URL: "http://localhost:8080/"
+  APP_ID: "nc_py_api"
+  APP_PORT: 9009
+  APP_VERSION: "1.0.0"
+  APP_SECRET: "tC6vkwPhcppjMykD1r0n9NlI95uJMBYjs5blpIcA1PAdoPDmc5qoAjaBAkyocZ6E"
+  SKIP_NC_CLIENT_TESTS: 1
+
 jobs:
   nc-py-api-pgsql:
     runs-on: ubuntu-22.04
@@ -27,13 +35,6 @@ jobs:
             php-version: "8.2"
           - server-version: "master"
             php-version: "8.3"
-    env:
-      NEXTCLOUD_URL: "http://localhost:8080/"
-      APP_ID: "nc_py_api"
-      APP_PORT: 9009
-      APP_VERSION: "1.0.0"
-      APP_SECRET: "tC6vkwPhcppjMykD1r0n9NlI95uJMBYjs5blpIcA1PAdoPDmc5qoAjaBAkyocZ6E"
-      SKIP_NC_CLIENT_TESTS: 1
 
     services:
       postgres:
@@ -145,14 +146,6 @@ jobs:
     runs-on: ubuntu-22.04
     name: NC_Py_API • stable27 • 8.1 • MySQL
 
-    env:
-      NEXTCLOUD_URL: "http://localhost:8080/"
-      APP_ID: "nc_py_api"
-      APP_PORT: 9009
-      APP_VERSION: "1.0.0"
-      APP_SECRET: "tC6vkwPhcppjMykD1r0n9NlI95uJMBYjs5blpIcA1PAdoPDmc5qoAjaBAkyocZ6E"
-      SKIP_NC_CLIENT_TESTS: 1
-
     services:
       mysql:
         image: ghcr.io/nextcloud/continuous-integration-mysql-8.1:latest

diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -7,6 +7,12 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 
 ## [Unreleased]
 
+## [1.3.0 - 2023-12-0x]
+
+### Changed
+
+- Reworked: `app_api:app:unregister` occ cli command, make it much robust. #127
+
 ## [1.2.2 - 2023-11-13]
 
 ### Fixed

diff --git a/composer.json b/composer.json
@@ -48,5 +48,8 @@
 		"psr-4": {
 			"OCP\\": "vendor/nextcloud/ocp/OCP"
 		}
-	}
+	},
+  "require": {
+    "ext-simplexml": "*"
+  }
 }
diff --git a/docs/ManagingExternalApplications.rst b/docs/ManagingExternalApplications.rst
@@ -65,10 +65,10 @@ Options
 Unregister
 ----------
 
-Command: ``app_api:app:unregister [--silent] [--rm-container] [--rm-data] [--] <appid>``
+Command: ``app_api:app:unregister [--keep-data] [--force] [--silent] [--] <appid>``
 
 To remove an ExApp you can use the unregister command.
-There are additional options to remove the ExApp container and persistent storage (data volume).
+There are additional options to keep the ExApp persistent storage (data volume).
 
 Arguments
 *********
@@ -78,9 +78,9 @@ Arguments
 Options
 *******
 
-    * ``--silent`` *[optional]* - do not disable ExApp before unregister
-    * ``--rm-container`` *[optional]* - remove ExApp container
-    * ``--rm-data`` *[optional]* - remove ExApp persistent storage (data volume)
+    * ``--keep-data`` *[optional]* - keep ExApp persistent storage (data volume)
+	* ``--force`` *[optional]* - continue removal even if some error occurs.
+	* ``--silent`` *[optional]* - print a minimum of information, display only some errors, if any.
 
 Update
 ------

diff --git a/lib/Command/ExApp/Unregister.php b/lib/Command/ExApp/Unregister.php
@@ -30,67 +30,104 @@ protected function configure(): void {
 
 		$this->addArgument('appid', InputArgument::REQUIRED);
 
-		$this->addOption('silent', null, InputOption::VALUE_NONE, 'Unregister only from Nextcloud. Do not send request to external app.');
-		$this->addOption('rm-container', null, InputOption::VALUE_NONE, 'Remove ExApp container');
-		$this->addOption('rm-data', null, InputOption::VALUE_NONE, 'Remove ExApp data (volume)');
+		$this->addOption(
+			'silent',
+			null,
+			InputOption::VALUE_NONE,
+			'Print only minimum and only errors.');
+		$this->addOption(
+			'force',
+			null,
+			InputOption::VALUE_NONE,
+			'Continue removal even if errors.');
+		$this->addOption('keep-data', null, InputOption::VALUE_NONE, 'Keep ExApp data (volume)');
 
 		$this->addUsage('test_app');
 		$this->addUsage('test_app --silent');
-		$this->addUsage('test_app --rm');
+		$this->addUsage('test_app --keep-data');
+		$this->addUsage('test_app --silent --force --keep-data');
 	}
 
 	protected function execute(InputInterface $input, OutputInterface $output): int {
 		$appId = $input->getArgument('appid');
+		$silent = $input->getOption('silent');
+		$force = $input->getOption('force');
+		$keep_data = $input->getOption('keep-data');
 
 		$exApp = $this->service->getExApp($appId);
 		if ($exApp === null) {
+			if ($silent) {
+				return 0;
+			}
 			$output->writeln(sprintf('ExApp %s not found. Failed to unregister.', $appId));
 			return 1;
 		}
 
-		$silent = $input->getOption('silent');
-
-		if (!$silent) {
-			if ($this->service->disableExApp($exApp)) {
+		if ($exApp->getEnabled()) {
+			if (!$this->service->disableExApp($exApp)) {
+				if (!$silent) {
+					$output->writeln(sprintf('Error during disabling %s ExApp.', $appId));
+				}
+				if (!$force) {
+					return 1;
+				}
+			} elseif (!$silent) {
 				$output->writeln(sprintf('ExApp %s successfully disabled.', $appId));
-			} else {
-				$output->writeln(sprintf('ExApp %s not disabled. Failed to disable.', $appId));
-				return 1;
 			}
 		}
 
-		$exApp = $this->service->unregisterExApp($appId);
-		if ($exApp === null) {
-			$output->writeln(sprintf('Failed to unregister ExApp %s.', $appId));
-			return 1;
-		}
-
-		$rmContainer = $input->getOption('rm-container');
-		if ($rmContainer) {
-			$daemonConfig = $this->daemonConfigService->getDaemonConfigByName($exApp->getDaemonConfigName());
-			if ($daemonConfig === null) {
-				$output->writeln(sprintf('Failed to get ExApp %s DaemonConfig by name %s', $appId, $exApp->getDaemonConfigName()));
+		$daemonConfig = $this->daemonConfigService->getDaemonConfigByName($exApp->getDaemonConfigName());
+		if ($daemonConfig === null) {
+			if (!$silent) {
+				$output->writeln(
+					sprintf('Failed to get ExApp %s DaemonConfig by name %s', $appId, $exApp->getDaemonConfigName())
+				);
+			}
+			if (!$force) {
 				return 1;
 			}
-			if ($daemonConfig->getAcceptsDeployId() === $this->dockerActions->getAcceptsDeployId()) {
-				$this->dockerActions->initGuzzleClient($daemonConfig);
-				[$stopResult, $removeResult] = $this->dockerActions->removePrevExAppContainer($this->dockerActions->buildDockerUrl($daemonConfig), $this->dockerActions->buildExAppContainerName($appId));
-				if (isset($stopResult['error']) || isset($removeResult['error'])) {
+		}
+		if ($daemonConfig->getAcceptsDeployId() === $this->dockerActions->getAcceptsDeployId()) {
+			$this->dockerActions->initGuzzleClient($daemonConfig);
+			[$stopResult, $removeResult] = $this->dockerActions->removePrevExAppContainer(
+				$this->dockerActions->buildDockerUrl($daemonConfig), $this->dockerActions->buildExAppContainerName($appId)
+			);
+			if (isset($stopResult['error']) || isset($removeResult['error'])) {
+				if (!$silent) {
 					$output->writeln(sprintf('Failed to remove ExApp %s container', $appId));
-				} else {
-					$rmData = $input->getOption('rm-data');
-					if ($rmData) {
-						$removeVolumeResult = $this->dockerActions->removeVolume($this->dockerActions->buildDockerUrl($daemonConfig), $this->dockerActions->buildExAppVolumeName($appId));
-						if (isset($removeVolumeResult['error'])) {
-							$output->writeln(sprintf('Failed to remove ExApp %s volume %s', $appId, $appId . '_data'));
-						}
+				}
+				if (!$force) {
+					return 1;
+				}
+			} elseif (!$silent) {
+				$output->writeln(sprintf('ExApp %s container successfully removed', $appId));
+			}
+			if (!$keep_data) {
+				$volumeName = $this->dockerActions->buildExAppVolumeName($appId);
+				$removeVolumeResult = $this->dockerActions->removeVolume(
+					$this->dockerActions->buildDockerUrl($daemonConfig), $volumeName
+				);
+				if (!$silent) {
+					if (isset($removeVolumeResult['error'])) {
+						$output->writeln(sprintf('Failed to remove ExApp %s volume: %s', $appId, $volumeName));
+					} else {
+						$output->writeln(sprintf('ExApp %s data volume successfully removed', $appId));
 					}
-					$output->writeln(sprintf('ExApp %s container successfully removed', $appId));
 				}
 			}
 		}
 
-		$output->writeln(sprintf('ExApp %s successfully unregistered.', $appId));
+		if ($this->service->unregisterExApp($appId) === null) {
+			if (!$silent) {
+				$output->writeln(sprintf('Failed to unregister ExApp %s.', $appId));
+			}
+			if (!$force) {
+				return 1;
+			}
+		}
+		if (!$silent) {
+			$output->writeln(sprintf('ExApp %s successfully unregistered.', $appId));
+		}
 		return 0;
 	}
 }