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 <vincent@nextcloud.com>
This commit is contained in:
Vincent Petry 2021-08-05 10:42:38 +02:00
parent 7f69a60ab1
commit 15a62c5237
No known key found for this signature in database
GPG key ID: E055D6A4D513575C
7 changed files with 72 additions and 5 deletions

View file

@ -5,7 +5,7 @@
<name>Nextcloud workflow engine</name>
<summary>Nextcloud workflow engine</summary>
<description>Nextcloud workflow engine</description>
<version>2.4.0</version>
<version>2.4.1</version>
<licence>agpl</licence>
<author>Arthur Schiwon</author>
<author>Julius Härtl</author>

View file

@ -32,6 +32,7 @@ return array(
'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',

View file

@ -47,6 +47,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',

View file

@ -36,20 +36,26 @@ use OCP\AppFramework\OCS\OCSException;
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;
}
/**
@ -115,6 +121,7 @@ abstract class AWorkflowController extends OCSController {
} catch (\DomainException $e) {
throw new OCSForbiddenException($e->getMessage(), $e);
} catch (Exception $e) {
$this->logger->error('Error when inserting flow', ['exception' => $e]);
throw new OCSException('An internal error occurred', $e->getCode(), $e);
}
}
@ -142,6 +149,7 @@ abstract class AWorkflowController extends OCSController {
} catch (\DomainException $e) {
throw new OCSForbiddenException($e->getMessage(), $e);
} catch (Exception $e) {
$this->logger->error('Error when updating flow with id ' . $id, ['exception' => $e]);
throw new OCSException('An internal error occurred', $e->getCode(), $e);
}
}
@ -160,6 +168,7 @@ abstract class AWorkflowController extends OCSController {
} catch (\DomainException $e) {
throw new OCSForbiddenException($e->getMessage(), $e);
} catch (Exception $e) {
$this->logger->error('Error when deleting flow with id ' . $id, ['exception' => $e]);
throw new OCSException('An internal error occurred', $e->getCode(), $e);
}
}

View file

@ -35,6 +35,7 @@ use OCP\AppFramework\OCS\OCSForbiddenException;
use OCP\IRequest;
use OCP\IUserSession;
use OCP\WorkflowEngine\IManager;
use Psr\Log\LoggerInterface;
class UserWorkflowsController extends AWorkflowController {
@ -48,9 +49,10 @@ class UserWorkflowsController extends AWorkflowController {
$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;
}

View file

@ -91,7 +91,7 @@ class Version2000Date20190808074233 extends SimpleMigrationStep {
'default' => '',
]);
$table->addColumn('name', Types::STRING, [
'notnull' => true,
'notnull' => false,
'length' => 256,
'default' => '',
]);

View file

@ -0,0 +1,54 @@
<?php
declare(strict_types=1);
/**
* @copyright Copyright (c) 2021 Vincent Petry <vincent@nextcloud.com>
*
* @author Vincent Petry <vincent@nextcloud.com>
*
* @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 <http://www.gnu.org/licenses/>.
*
*/
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;
}
}