From ffe7467187d7e64fa75fd12fe3429455061c553c Mon Sep 17 00:00:00 2001 From: Vincent Petry Date: Thu, 5 Aug 2021 10:42:38 +0200 Subject: [PATCH] Make "name" column nullable in workflow operations The "name" column is now unused and the code is always inserting an empty string. While this works with most databases, Oracle complains because an empty string is equivalent to null. To fix this, the column definition is changed to allow null values now. Also added some logging in case of database exceptions, because without this nothing would be logged to detect the above problem. Signed-off-by: Vincent Petry --- apps/workflowengine/appinfo/info.xml | 2 +- .../composer/composer/autoload_classmap.php | 1 + .../composer/composer/autoload_static.php | 1 + .../lib/Controller/AWorkflowController.php | 11 +++- .../Controller/UserWorkflowsController.php | 6 ++- .../Version2000Date20190808074233.php | 2 +- .../Version2200Date20210805101925.php | 54 +++++++++++++++++++ 7 files changed, 72 insertions(+), 5 deletions(-) create mode 100644 apps/workflowengine/lib/Migration/Version2200Date20210805101925.php diff --git a/apps/workflowengine/appinfo/info.xml b/apps/workflowengine/appinfo/info.xml index 5a6cd8b7b2ad8..18fcd1923a450 100644 --- a/apps/workflowengine/appinfo/info.xml +++ b/apps/workflowengine/appinfo/info.xml @@ -5,7 +5,7 @@ Nextcloud workflow engine Nextcloud workflow engine Nextcloud workflow engine - 2.2.0 + 2.2.1 agpl Arthur Schiwon Julius Härtl diff --git a/apps/workflowengine/composer/composer/autoload_classmap.php b/apps/workflowengine/composer/composer/autoload_classmap.php index cb2f38372984f..eb26131a48bee 100644 --- a/apps/workflowengine/composer/composer/autoload_classmap.php +++ b/apps/workflowengine/composer/composer/autoload_classmap.php @@ -31,6 +31,7 @@ 'OCA\\WorkflowEngine\\Manager' => $baseDir . '/../lib/Manager.php', 'OCA\\WorkflowEngine\\Migration\\PopulateNewlyIntroducedDatabaseFields' => $baseDir . '/../lib/Migration/PopulateNewlyIntroducedDatabaseFields.php', 'OCA\\WorkflowEngine\\Migration\\Version2000Date20190808074233' => $baseDir . '/../lib/Migration/Version2000Date20190808074233.php', + 'OCA\\WorkflowEngine\\Migration\\Version2200Date20210805101925' => $baseDir . '/../lib/Migration/Version2200Date20210805101925.php', 'OCA\\WorkflowEngine\\Service\\Logger' => $baseDir . '/../lib/Service/Logger.php', 'OCA\\WorkflowEngine\\Service\\RuleMatcher' => $baseDir . '/../lib/Service/RuleMatcher.php', 'OCA\\WorkflowEngine\\Settings\\ASettings' => $baseDir . '/../lib/Settings/ASettings.php', diff --git a/apps/workflowengine/composer/composer/autoload_static.php b/apps/workflowengine/composer/composer/autoload_static.php index c5bc8e28ba710..b39ecc80c040d 100644 --- a/apps/workflowengine/composer/composer/autoload_static.php +++ b/apps/workflowengine/composer/composer/autoload_static.php @@ -46,6 +46,7 @@ class ComposerStaticInitWorkflowEngine 'OCA\\WorkflowEngine\\Manager' => __DIR__ . '/..' . '/../lib/Manager.php', 'OCA\\WorkflowEngine\\Migration\\PopulateNewlyIntroducedDatabaseFields' => __DIR__ . '/..' . '/../lib/Migration/PopulateNewlyIntroducedDatabaseFields.php', 'OCA\\WorkflowEngine\\Migration\\Version2000Date20190808074233' => __DIR__ . '/..' . '/../lib/Migration/Version2000Date20190808074233.php', + 'OCA\\WorkflowEngine\\Migration\\Version2200Date20210805101925' => __DIR__ . '/..' . '/../lib/Migration/Version2200Date20210805101925.php', 'OCA\\WorkflowEngine\\Service\\Logger' => __DIR__ . '/..' . '/../lib/Service/Logger.php', 'OCA\\WorkflowEngine\\Service\\RuleMatcher' => __DIR__ . '/..' . '/../lib/Service/RuleMatcher.php', 'OCA\\WorkflowEngine\\Settings\\ASettings' => __DIR__ . '/..' . '/../lib/Settings/ASettings.php', diff --git a/apps/workflowengine/lib/Controller/AWorkflowController.php b/apps/workflowengine/lib/Controller/AWorkflowController.php index 6d109f7dccfc6..7a0108ee5e5db 100644 --- a/apps/workflowengine/lib/Controller/AWorkflowController.php +++ b/apps/workflowengine/lib/Controller/AWorkflowController.php @@ -34,20 +34,26 @@ use OCP\AppFramework\OCS\OCSForbiddenException; use OCP\AppFramework\OCSController; use OCP\IRequest; +use Psr\Log\LoggerInterface; abstract class AWorkflowController extends OCSController { /** @var Manager */ protected $manager; + /** @var LoggerInterface */ + private $logger; + public function __construct( $appName, IRequest $request, - Manager $manager + Manager $manager, + LoggerInterface $logger ) { parent::__construct($appName, $request); $this->manager = $manager; + $this->logger = $logger; } /** @@ -113,6 +119,7 @@ public function create( } catch (\DomainException $e) { throw new OCSForbiddenException($e->getMessage(), $e); } catch (DBALException $e) { + $this->logger->error('Error when inserting flow', ['exception' => $e]); throw new OCSException('An internal error occurred', $e->getCode(), $e); } } @@ -140,6 +147,7 @@ public function update( } catch (\DomainException $e) { throw new OCSForbiddenException($e->getMessage(), $e); } catch (DBALException $e) { + $this->logger->error('Error when updating flow with id ' . $id, ['exception' => $e]); throw new OCSException('An internal error occurred', $e->getCode(), $e); } } @@ -158,6 +166,7 @@ public function destroy(int $id): DataResponse { } catch (\DomainException $e) { throw new OCSForbiddenException($e->getMessage(), $e); } catch (DBALException $e) { + $this->logger->error('Error when deleting flow with id ' . $id, ['exception' => $e]); throw new OCSException('An internal error occurred', $e->getCode(), $e); } } diff --git a/apps/workflowengine/lib/Controller/UserWorkflowsController.php b/apps/workflowengine/lib/Controller/UserWorkflowsController.php index c3884b6197998..ce1ce05af9bfa 100644 --- a/apps/workflowengine/lib/Controller/UserWorkflowsController.php +++ b/apps/workflowengine/lib/Controller/UserWorkflowsController.php @@ -33,6 +33,7 @@ use OCP\IRequest; use OCP\IUserSession; use OCP\WorkflowEngine\IManager; +use Psr\Log\LoggerInterface; class UserWorkflowsController extends AWorkflowController { @@ -46,9 +47,10 @@ public function __construct( $appName, IRequest $request, Manager $manager, - IUserSession $session + IUserSession $session, + LoggerInterface $logger ) { - parent::__construct($appName, $request, $manager); + parent::__construct($appName, $request, $manager, $logger); $this->session = $session; } diff --git a/apps/workflowengine/lib/Migration/Version2000Date20190808074233.php b/apps/workflowengine/lib/Migration/Version2000Date20190808074233.php index 58458e84c86d0..6803c87a3a56f 100644 --- a/apps/workflowengine/lib/Migration/Version2000Date20190808074233.php +++ b/apps/workflowengine/lib/Migration/Version2000Date20190808074233.php @@ -66,7 +66,7 @@ public function changeSchema(IOutput $output, Closure $schemaClosure, array $opt 'default' => '', ]); $table->addColumn('name', Types::STRING, [ - 'notnull' => true, + 'notnull' => false, 'length' => 256, 'default' => '', ]); diff --git a/apps/workflowengine/lib/Migration/Version2200Date20210805101925.php b/apps/workflowengine/lib/Migration/Version2200Date20210805101925.php new file mode 100644 index 0000000000000..9f2049dc61143 --- /dev/null +++ b/apps/workflowengine/lib/Migration/Version2200Date20210805101925.php @@ -0,0 +1,54 @@ + + * + * @author Vincent Petry + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + * + */ +namespace OCA\WorkflowEngine\Migration; + +use Closure; +use OCP\DB\ISchemaWrapper; +use OCP\Migration\IOutput; +use OCP\Migration\SimpleMigrationStep; + +class Version2200Date20210805101925 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('flow_operations')) { + $table = $schema->getTable('flow_operations'); + $table->changeColumn('name', [ + 'notnull' => false, + ]); + } + + return $schema; + } +}