Skip to content

Commit

Permalink
do not disable ExApp on lower version (#341)
Browse files Browse the repository at this point in the history
1. Will be required for future AI ExApps with scaling
2. Will be required for future ExApps with federated support
3. Requited by **WorkflowEngine project** at it's current stage
4. This will make developing of ExApps easier, as usual in
"manual"registration" we use different app version(constant "1.0.0")

---------

Signed-off-by: Alexander Piskun <[email protected]>
  • Loading branch information
bigcat88 authored Aug 1, 2024
1 parent b204a73 commit f43ad47
Show file tree
Hide file tree
Showing 7 changed files with 8 additions and 234 deletions.
114 changes: 1 addition & 113 deletions .github/workflows/tests-special.yml
Original file line number Diff line number Diff line change
Expand Up @@ -21,118 +21,6 @@ env:
APP_SECRET: "tC6vkwPhcppjMykD1r0n9NlI95uJMBYjs5blpIcA1PAdoPDmc5qoAjaBAkyocZ6E"

jobs:
app-version-higher:
runs-on: ubuntu-22.04
name: ExApp version higher

services:
postgres:
image: ghcr.io/nextcloud/continuous-integration-postgres-14:latest
ports:
- 4444:5432/tcp
env:
POSTGRES_USER: root
POSTGRES_PASSWORD: rootpassword
POSTGRES_DB: nextcloud
options: --health-cmd pg_isready --health-interval 5s --health-timeout 2s --health-retries 5

steps:
- uses: actions/setup-python@v4
with:
python-version: '3.11'

- name: Set app env
run: echo "APP_NAME=${GITHUB_REPOSITORY##*/}" >> $GITHUB_ENV

- name: Checkout server
uses: actions/checkout@c85c95e3d7251135ab7dc9ce3241c5835cc595a9 # v3.5.3
with:
submodules: true
repository: nextcloud/server
ref: 'stable28'

- name: Checkout Notifications
uses: actions/checkout@v3
with:
repository: nextcloud/notifications
ref: 'stable28'
path: apps/notifications

- name: Checkout AppAPI
uses: actions/checkout@c85c95e3d7251135ab7dc9ce3241c5835cc595a9 # v3.5.3
with:
path: apps/${{ env.APP_NAME }}

- name: Set up php
uses: shivammathur/setup-php@4bd44f22a98a19e0950cbad5f31095157cc9621b # v2
with:
php-version: '8.1'
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
env:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}

- name: Check composer file existence
id: check_composer
uses: andstor/file-existence-action@20b4d2e596410855db8f9ca21e96fbe18e12930b # v2
with:
files: apps/${{ env.APP_NAME }}/composer.json

- name: Set up dependencies
if: steps.check_composer.outputs.files_exists == 'true'
working-directory: apps/${{ env.APP_NAME }}
run: composer i

- name: Set up Nextcloud
env:
DB_PORT: 4444
run: |
mkdir data
./occ maintenance:install --verbose --database=pgsql --database-name=nextcloud --database-host=127.0.0.1 \
--database-port=$DB_PORT --database-user=root --database-pass=rootpassword \
--admin-user admin --admin-pass admin
./occ app:enable notifications
./occ app:enable --force ${{ env.APP_NAME }}
- name: Run Nextcloud
run: PHP_CLI_SERVER_WORKERS=2 php -S 127.0.0.1:8080 &

- name: Checkout NcPyApi
uses: actions/checkout@v3
with:
path: nc_py_api
repository: cloud-py-api/nc_py_api

- name: Install NcPyApi
working-directory: nc_py_api
run: python3 -m pip -v install ".[dev]"

- name: Register NcPyApi
run: |
cd nc_py_api
python3 tests/_install.py &
echo $! > /tmp/_install.pid
cd ..
sleep 5s
php occ app_api:daemon:register manual_install "Manual Install" manual-install http localhost 0
php occ app_api:app:register $APP_ID manual_install --json-info \
"{\"appid\":\"$APP_ID\",\"name\":\"$APP_ID\",\"daemon_config_name\":\"manual_install\",\"version\":\"$APP_VERSION\",\"secret\":\"$APP_SECRET\",\"port\":$APP_PORT,\"scopes\":[\"SYSTEM\", \"NOTIFICATIONS\", \"USER_INFO\"],\"system_app\":1}" \
--force-scopes --wait-finish
kill -15 $(cat /tmp/_install.pid)
timeout 3m tail --pid=$(cat /tmp/_install.pid) -f /dev/null
- name: Run Manual App Update test
working-directory: apps/${{ env.APP_NAME }}
run: python3 tests/app_version_higher.py

- name: Upload NC logs
if: always()
uses: actions/upload-artifact@v3
with:
name: app_version_higher_nextcloud.log
path: data/nextcloud.log
if-no-files-found: warn

auth-tests-no-init:
runs-on: ubuntu-22.04
Expand Down Expand Up @@ -258,7 +146,7 @@ jobs:
permissions:
contents: none
runs-on: ubuntu-22.04
needs: [app-version-higher, auth-tests-no-init]
needs: [auth-tests-no-init]
name: TestsSpecial-OK
steps:
- run: echo "Tests special passed successfully"
2 changes: 0 additions & 2 deletions lib/AppInfo/Application.php
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
use OCA\AppAPI\Middleware\AppAPIAuthMiddleware;
use OCA\AppAPI\Middleware\ExAppUIL10NMiddleware;
use OCA\AppAPI\Middleware\ExAppUiMiddleware;
use OCA\AppAPI\Notifications\ExAppAdminNotifier;
use OCA\AppAPI\Notifications\ExAppNotifier;
use OCA\AppAPI\PublicCapabilities;
use OCA\AppAPI\Service\ProvidersAI\SpeechToTextService;
Expand Down Expand Up @@ -74,7 +73,6 @@ public function register(IRegistrationContext $context): void {
$context->registerMiddleware(ExAppUIL10NMiddleware::class, true);
$context->registerEventListener(SabrePluginAuthInitEvent::class, SabrePluginAuthInitListener::class);
$context->registerNotifierService(ExAppNotifier::class);
$context->registerNotifierService(ExAppAdminNotifier::class);

$context->registerEventListener(DeclarativeSettingsRegisterFormEvent::class, RegisterDeclarativeSettingsListener::class);
$context->registerEventListener(DeclarativeSettingsGetValueEvent::class, GetValueListener::class);
Expand Down
58 changes: 0 additions & 58 deletions lib/Notifications/ExAppAdminNotifier.php

This file was deleted.

4 changes: 1 addition & 3 deletions lib/Notifications/ExAppNotifier.php
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,7 @@ public function prepare(INotification $notification, string $languageCode): INot
if ($exApp === null) {
throw new InvalidArgumentException();
}
if ($notification->getSubject() === 'ex_app_version_update' && $exApp->getEnabled()) {
throw new InvalidArgumentException('ExApp is probably already re-enabled');
} elseif (!$exApp->getEnabled()) { // Only enabled ExApps can render notifications
if (!$exApp->getEnabled()) { // Only enabled ExApps can render notifications
throw new InvalidArgumentException('ExApp is disabled');
}

Expand Down
36 changes: 5 additions & 31 deletions lib/Service/AppAPIService.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
use OCA\AppAPI\Db\ExApp;
use OCA\AppAPI\DeployActions\DockerActions;
use OCA\AppAPI\DeployActions\ManualActions;
use OCA\AppAPI\Notifications\ExNotificationsManager;
use OCP\AppFramework\Http;
use OCP\DB\Exception;
use OCP\Http\Client\IClient;
Expand All @@ -36,7 +35,6 @@ public function __construct(
private readonly ISession $session,
private readonly IUserManager $userManager,
private readonly IFactory $l10nFactory,
private readonly ExNotificationsManager $exNotificationsManager,
private readonly ExAppService $exAppService,
private readonly ExAppApiScopeService $exAppApiScopeService,
private readonly DockerActions $dockerActions,
Expand Down Expand Up @@ -400,42 +398,18 @@ public function passesScopeCheck(ExApp $exApp, int $apiScope): bool {
}

/**
* Check if ExApp version changed and update it in database.
* Immediately disable ExApp and send notifications to the administrators (users of admins group).
* This handling only intentional case of manual ExApp update
* so the administrator must re-enable ExApp in UI or CLI after that.
*
* Ref: https://github.com/cloud-py-api/app_api/pull/29
* TODO: Add link to docs with warning and mark as not-recommended
*
* Checks if the ExApp version changed and if it is higher, updates it in the database.
*/
public function handleExAppVersionChange(IRequest $request, ExApp $exApp): bool {
$requestExAppVersion = $request->getHeader('EX-APP-VERSION');
$versionValid = $exApp->getVersion() === $requestExAppVersion;
if (!$versionValid) {
// Update ExApp version
$oldVersion = $exApp->getVersion();
if ($requestExAppVersion === '') {
return false;
}
if (version_compare($requestExAppVersion, $exApp->getVersion(), '>')) {
$exApp->setVersion($requestExAppVersion);
if (!$this->exAppService->updateExApp($exApp, ['version'])) {
return false;
}
$this->disableExApp($exApp);
$this->exNotificationsManager->sendAdminsNotification($exApp->getAppid(), [
'object' => 'ex_app_update',
'object_id' => $exApp->getAppid(),
'subject_type' => 'ex_app_version_update',
'subject_params' => [
'rich_subject' => 'ExApp updated, action required!',
'rich_subject_params' => [],
'rich_message' => sprintf(
'ExApp %s disabled due to update from %s to %s. Manual re-enable required.',
$exApp->getAppid(),
$oldVersion,
$exApp->getVersion()),
'rich_message_params' => [],
],
]);
return false;
}
return true;
}
Expand Down
2 changes: 1 addition & 1 deletion lib/Service/ExAppService.php
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,7 @@ public function updateExApp(ExApp $exApp, array $fields = ['version', 'name', 'p
try {
$this->exAppMapper->updateExApp($exApp, $fields);
$this->cache->remove('/ex_apps');
if (in_array('enabled', $fields)) {
if (in_array('enabled', $fields) || in_array('version', $fields)) {
$this->resetCaches();
}
return true;
Expand Down
26 changes: 0 additions & 26 deletions tests/app_version_higher.py

This file was deleted.

0 comments on commit f43ad47

Please sign in to comment.