Skip to content

Commit 0f12316

Browse files
PVince81backportbot[bot]
authored andcommitted
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>
1 parent 514ba5f commit 0f12316

File tree

6 files changed

+71
-4
lines changed

6 files changed

+71
-4
lines changed

apps/workflowengine/composer/composer/autoload_classmap.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232
'OCA\\WorkflowEngine\\Manager' => $baseDir . '/../lib/Manager.php',
3333
'OCA\\WorkflowEngine\\Migration\\PopulateNewlyIntroducedDatabaseFields' => $baseDir . '/../lib/Migration/PopulateNewlyIntroducedDatabaseFields.php',
3434
'OCA\\WorkflowEngine\\Migration\\Version2000Date20190808074233' => $baseDir . '/../lib/Migration/Version2000Date20190808074233.php',
35+
'OCA\\WorkflowEngine\\Migration\\Version2200Date20210805101925' => $baseDir . '/../lib/Migration/Version2200Date20210805101925.php',
3536
'OCA\\WorkflowEngine\\Service\\Logger' => $baseDir . '/../lib/Service/Logger.php',
3637
'OCA\\WorkflowEngine\\Service\\RuleMatcher' => $baseDir . '/../lib/Service/RuleMatcher.php',
3738
'OCA\\WorkflowEngine\\Settings\\ASettings' => $baseDir . '/../lib/Settings/ASettings.php',

apps/workflowengine/composer/composer/autoload_static.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ class ComposerStaticInitWorkflowEngine
4747
'OCA\\WorkflowEngine\\Manager' => __DIR__ . '/..' . '/../lib/Manager.php',
4848
'OCA\\WorkflowEngine\\Migration\\PopulateNewlyIntroducedDatabaseFields' => __DIR__ . '/..' . '/../lib/Migration/PopulateNewlyIntroducedDatabaseFields.php',
4949
'OCA\\WorkflowEngine\\Migration\\Version2000Date20190808074233' => __DIR__ . '/..' . '/../lib/Migration/Version2000Date20190808074233.php',
50+
'OCA\\WorkflowEngine\\Migration\\Version2200Date20210805101925' => __DIR__ . '/..' . '/../lib/Migration/Version2200Date20210805101925.php',
5051
'OCA\\WorkflowEngine\\Service\\Logger' => __DIR__ . '/..' . '/../lib/Service/Logger.php',
5152
'OCA\\WorkflowEngine\\Service\\RuleMatcher' => __DIR__ . '/..' . '/../lib/Service/RuleMatcher.php',
5253
'OCA\\WorkflowEngine\\Settings\\ASettings' => __DIR__ . '/..' . '/../lib/Settings/ASettings.php',

apps/workflowengine/lib/Controller/AWorkflowController.php

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,20 +34,26 @@
3434
use OCP\AppFramework\OCS\OCSForbiddenException;
3535
use OCP\AppFramework\OCSController;
3636
use OCP\IRequest;
37+
use Psr\Log\LoggerInterface;
3738

3839
abstract class AWorkflowController extends OCSController {
3940

4041
/** @var Manager */
4142
protected $manager;
4243

44+
/** @var LoggerInterface */
45+
private $logger;
46+
4347
public function __construct(
4448
$appName,
4549
IRequest $request,
46-
Manager $manager
50+
Manager $manager,
51+
LoggerInterface $logger
4752
) {
4853
parent::__construct($appName, $request);
4954

5055
$this->manager = $manager;
56+
$this->logger = $logger;
5157
}
5258

5359
/**
@@ -113,6 +119,7 @@ public function create(
113119
} catch (\DomainException $e) {
114120
throw new OCSForbiddenException($e->getMessage(), $e);
115121
} catch (Exception $e) {
122+
$this->logger->error('Error when inserting flow', ['exception' => $e]);
116123
throw new OCSException('An internal error occurred', $e->getCode(), $e);
117124
}
118125
}
@@ -140,6 +147,7 @@ public function update(
140147
} catch (\DomainException $e) {
141148
throw new OCSForbiddenException($e->getMessage(), $e);
142149
} catch (Exception $e) {
150+
$this->logger->error('Error when updating flow with id ' . $id, ['exception' => $e]);
143151
throw new OCSException('An internal error occurred', $e->getCode(), $e);
144152
}
145153
}
@@ -158,6 +166,7 @@ public function destroy(int $id): DataResponse {
158166
} catch (\DomainException $e) {
159167
throw new OCSForbiddenException($e->getMessage(), $e);
160168
} catch (Exception $e) {
169+
$this->logger->error('Error when deleting flow with id ' . $id, ['exception' => $e]);
161170
throw new OCSException('An internal error occurred', $e->getCode(), $e);
162171
}
163172
}

apps/workflowengine/lib/Controller/UserWorkflowsController.php

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333
use OCP\IRequest;
3434
use OCP\IUserSession;
3535
use OCP\WorkflowEngine\IManager;
36+
use Psr\Log\LoggerInterface;
3637

3738
class UserWorkflowsController extends AWorkflowController {
3839

@@ -46,9 +47,10 @@ public function __construct(
4647
$appName,
4748
IRequest $request,
4849
Manager $manager,
49-
IUserSession $session
50+
IUserSession $session,
51+
LoggerInterface $logger
5052
) {
51-
parent::__construct($appName, $request, $manager);
53+
parent::__construct($appName, $request, $manager, $logger);
5254

5355
$this->session = $session;
5456
}

apps/workflowengine/lib/Migration/Version2000Date20190808074233.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ public function changeSchema(IOutput $output, Closure $schemaClosure, array $opt
6666
'default' => '',
6767
]);
6868
$table->addColumn('name', Types::STRING, [
69-
'notnull' => true,
69+
'notnull' => false,
7070
'length' => 256,
7171
'default' => '',
7272
]);
Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
/**
6+
* @copyright Copyright (c) 2021 Vincent Petry <vincent@nextcloud.com>
7+
*
8+
* @author Vincent Petry <vincent@nextcloud.com>
9+
*
10+
* @license GNU AGPL version 3 or any later version
11+
*
12+
* This program is free software: you can redistribute it and/or modify
13+
* it under the terms of the GNU Affero General Public License as
14+
* published by the Free Software Foundation, either version 3 of the
15+
* License, or (at your option) any later version.
16+
*
17+
* This program is distributed in the hope that it will be useful,
18+
* but WITHOUT ANY WARRANTY; without even the implied warranty of
19+
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
20+
* GNU Affero General Public License for more details.
21+
*
22+
* You should have received a copy of the GNU Affero General Public License
23+
* along with this program. If not, see <http://www.gnu.org/licenses/>.
24+
*
25+
*/
26+
namespace OCA\WorkflowEngine\Migration;
27+
28+
use Closure;
29+
use OCP\DB\ISchemaWrapper;
30+
use OCP\Migration\IOutput;
31+
use OCP\Migration\SimpleMigrationStep;
32+
33+
class Version2200Date20210805101925 extends SimpleMigrationStep {
34+
35+
/**
36+
* @param IOutput $output
37+
* @param Closure $schemaClosure The `\Closure` returns a `ISchemaWrapper`
38+
* @param array $options
39+
* @return null|ISchemaWrapper
40+
*/
41+
public function changeSchema(IOutput $output, Closure $schemaClosure, array $options) {
42+
/** @var ISchemaWrapper $schema */
43+
$schema = $schemaClosure();
44+
45+
if ($schema->hasTable('flow_operations')) {
46+
$table = $schema->getTable('flow_operations');
47+
$table->changeColumn('name', [
48+
'notnull' => false,
49+
]);
50+
}
51+
52+
return $schema;
53+
}
54+
}

0 commit comments

Comments
 (0)