Skip to content

Commit b707513

Browse files
committed
refactor(workflowengine): Port away from old events
And enforce via static analysis that only new event classes are used. We have to also create a new private event for the workflowengine as the rule matcher is not able to handle multiple nodes at the same time. Signed-off-by: Carl Schwan <[email protected]>
1 parent 3e78bf6 commit b707513

File tree

18 files changed

+380
-114
lines changed

18 files changed

+380
-114
lines changed

apps/workflowengine/appinfo/info.xml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
<name>Nextcloud workflow engine</name>
1010
<summary>Nextcloud workflow engine</summary>
1111
<description>Nextcloud workflow engine</description>
12-
<version>2.16.0</version>
12+
<version>2.16.1</version>
1313
<licence>agpl</licence>
1414
<author>Arthur Schiwon</author>
1515
<author>Julius Härtl</author>

apps/workflowengine/composer/composer/autoload_classmap.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333
'OCA\\WorkflowEngine\\Migration\\PopulateNewlyIntroducedDatabaseFields' => $baseDir . '/../lib/Migration/PopulateNewlyIntroducedDatabaseFields.php',
3434
'OCA\\WorkflowEngine\\Migration\\Version2000Date20190808074233' => $baseDir . '/../lib/Migration/Version2000Date20190808074233.php',
3535
'OCA\\WorkflowEngine\\Migration\\Version2200Date20210805101925' => $baseDir . '/../lib/Migration/Version2200Date20210805101925.php',
36+
'OCA\\WorkflowEngine\\Migration\\Version3400Date20260227000000' => $baseDir . '/../lib/Migration/Version3400Date20260227000000.php',
3637
'OCA\\WorkflowEngine\\ResponseDefinitions' => $baseDir . '/../lib/ResponseDefinitions.php',
3738
'OCA\\WorkflowEngine\\Service\\Logger' => $baseDir . '/../lib/Service/Logger.php',
3839
'OCA\\WorkflowEngine\\Service\\RuleMatcher' => $baseDir . '/../lib/Service/RuleMatcher.php',

apps/workflowengine/composer/composer/autoload_static.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@ class ComposerStaticInitWorkflowEngine
4848
'OCA\\WorkflowEngine\\Migration\\PopulateNewlyIntroducedDatabaseFields' => __DIR__ . '/..' . '/../lib/Migration/PopulateNewlyIntroducedDatabaseFields.php',
4949
'OCA\\WorkflowEngine\\Migration\\Version2000Date20190808074233' => __DIR__ . '/..' . '/../lib/Migration/Version2000Date20190808074233.php',
5050
'OCA\\WorkflowEngine\\Migration\\Version2200Date20210805101925' => __DIR__ . '/..' . '/../lib/Migration/Version2200Date20210805101925.php',
51+
'OCA\\WorkflowEngine\\Migration\\Version3400Date20260227000000' => __DIR__ . '/..' . '/../lib/Migration/Version3400Date20260227000000.php',
5152
'OCA\\WorkflowEngine\\ResponseDefinitions' => __DIR__ . '/..' . '/../lib/ResponseDefinitions.php',
5253
'OCA\\WorkflowEngine\\Service\\Logger' => __DIR__ . '/..' . '/../lib/Service/Logger.php',
5354
'OCA\\WorkflowEngine\\Service\\RuleMatcher' => __DIR__ . '/..' . '/../lib/Service/RuleMatcher.php',

apps/workflowengine/lib/Entity/File.php

Lines changed: 60 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,15 @@
99
namespace OCA\WorkflowEngine\Entity;
1010

1111
use OC\Files\Config\UserMountCache;
12+
use OC\SystemTag\Events\SingleTagAssignedEvent;
1213
use OCP\EventDispatcher\Event;
13-
use OCP\EventDispatcher\GenericEvent;
14+
use OCP\Files\Events\Node\AbstractNodeEvent;
15+
use OCP\Files\Events\Node\NodeCopiedEvent;
16+
use OCP\Files\Events\Node\NodeCreatedEvent;
17+
use OCP\Files\Events\Node\NodeDeletedEvent;
18+
use OCP\Files\Events\Node\NodeRenamedEvent;
19+
use OCP\Files\Events\Node\NodeTouchedEvent;
20+
use OCP\Files\Events\Node\NodeUpdatedEvent;
1421
use OCP\Files\InvalidPathException;
1522
use OCP\Files\IRootFolder;
1623
use OCP\Files\Mount\IMountManager;
@@ -22,78 +29,85 @@
2229
use OCP\IUserManager;
2330
use OCP\IUserSession;
2431
use OCP\SystemTag\ISystemTagManager;
25-
use OCP\SystemTag\MapperEvent;
2632
use OCP\WorkflowEngine\EntityContext\IContextPortation;
2733
use OCP\WorkflowEngine\EntityContext\IDisplayText;
2834
use OCP\WorkflowEngine\EntityContext\IIcon;
2935
use OCP\WorkflowEngine\EntityContext\IUrl;
3036
use OCP\WorkflowEngine\GenericEntityEvent;
3137
use OCP\WorkflowEngine\IEntity;
3238
use OCP\WorkflowEngine\IRuleMatcher;
39+
use Override;
3340

3441
class File implements IEntity, IDisplayText, IUrl, IIcon, IContextPortation {
35-
private const EVENT_NAMESPACE = '\OCP\Files::';
42+
/** @var ?class-string<Event> $eventName */
3643
protected ?string $eventName = null;
3744
protected ?Event $event = null;
3845
private ?Node $node = null;
3946
private ?IUser $actingUser = null;
4047

4148
public function __construct(
42-
protected IL10N $l10n,
43-
protected IURLGenerator $urlGenerator,
44-
protected IRootFolder $root,
45-
private IUserSession $userSession,
46-
private ISystemTagManager $tagManager,
47-
private IUserManager $userManager,
48-
private UserMountCache $userMountCache,
49-
private IMountManager $mountManager,
49+
protected readonly IL10N $l10n,
50+
protected readonly IURLGenerator $urlGenerator,
51+
protected readonly IRootFolder $root,
52+
private readonly IUserSession $userSession,
53+
private readonly ISystemTagManager $tagManager,
54+
private readonly IUserManager $userManager,
55+
private readonly UserMountCache $userMountCache,
56+
private readonly IMountManager $mountManager,
5057
) {
5158
}
5259

60+
#[Override]
5361
public function getName(): string {
5462
return $this->l10n->t('File');
5563
}
5664

65+
#[Override]
5766
public function getIcon(): string {
5867
return $this->urlGenerator->imagePath('core', 'categories/files.svg');
5968
}
6069

70+
#[Override]
6171
public function getEvents(): array {
6272
return [
63-
new GenericEntityEvent($this->l10n->t('File created'), self::EVENT_NAMESPACE . 'postCreate'),
64-
new GenericEntityEvent($this->l10n->t('File updated'), self::EVENT_NAMESPACE . 'postWrite'),
65-
new GenericEntityEvent($this->l10n->t('File renamed'), self::EVENT_NAMESPACE . 'postRename'),
66-
new GenericEntityEvent($this->l10n->t('File deleted'), self::EVENT_NAMESPACE . 'postDelete'),
67-
new GenericEntityEvent($this->l10n->t('File accessed'), self::EVENT_NAMESPACE . 'postTouch'),
68-
new GenericEntityEvent($this->l10n->t('File copied'), self::EVENT_NAMESPACE . 'postCopy'),
69-
new GenericEntityEvent($this->l10n->t('Tag assigned'), MapperEvent::EVENT_ASSIGN),
73+
new GenericEntityEvent($this->l10n->t('File created'), NodeCreatedEvent::class),
74+
new GenericEntityEvent($this->l10n->t('File updated'), NodeUpdatedEvent::class),
75+
new GenericEntityEvent($this->l10n->t('File renamed'), NodeRenamedEvent::class),
76+
new GenericEntityEvent($this->l10n->t('File deleted'), NodeDeletedEvent::class),
77+
new GenericEntityEvent($this->l10n->t('File accessed'), NodeTouchedEvent::class),
78+
new GenericEntityEvent($this->l10n->t('File copied'), NodeCopiedEvent::class),
79+
new GenericEntityEvent($this->l10n->t('Tag assigned'), SingleTagAssignedEvent::class),
7080
];
7181
}
7282

83+
#[Override]
7384
public function prepareRuleMatcher(IRuleMatcher $ruleMatcher, string $eventName, Event $event): void {
74-
if (!$event instanceof GenericEvent && !$event instanceof MapperEvent) {
85+
$isSupported = array_any($this->getEvents(), static fn (GenericEntityEvent $genericEvent): bool => is_a($event, $genericEvent->getEventName()));
86+
if (!$isSupported) {
7587
return;
7688
}
89+
7790
$this->eventName = $eventName;
7891
$this->event = $event;
7992
$this->actingUser = $this->actingUser ?? $this->userSession->getUser();
8093
try {
8194
$node = $this->getNode();
8295
$ruleMatcher->setEntitySubject($this, $node);
8396
$ruleMatcher->setFileInfo($node->getStorage(), $node->getInternalPath());
84-
} catch (NotFoundException $e) {
97+
} catch (NotFoundException) {
8598
// pass
8699
}
87100
}
88101

102+
#[Override]
89103
public function isLegitimatedForUserId(string $userId): bool {
90104
try {
91105
$node = $this->getNode();
92106
if ($node->getOwner()?->getUID() === $userId) {
93107
return true;
94108
}
95109

96-
if ($this->eventName === self::EVENT_NAMESPACE . 'postDelete') {
110+
if ($this->eventName === NodeDeletedEvent::class) {
97111
// At postDelete, the file no longer exists. Check for parent folder instead.
98112
$fileId = $node->getParentId();
99113
} else {
@@ -120,35 +134,27 @@ protected function getNode(): Node {
120134
if ($this->node) {
121135
return $this->node;
122136
}
123-
if (!$this->event instanceof GenericEvent && !$this->event instanceof MapperEvent) {
137+
138+
if ($this->event instanceof AbstractNodeEvent) {
139+
return $this->event->getNode();
140+
}
141+
142+
if (!$this->event instanceof SingleTagAssignedEvent || $this->event->getObjectType() !== 'files') {
124143
throw new NotFoundException();
125144
}
126-
switch ($this->eventName) {
127-
case self::EVENT_NAMESPACE . 'postCreate':
128-
case self::EVENT_NAMESPACE . 'postWrite':
129-
case self::EVENT_NAMESPACE . 'postDelete':
130-
case self::EVENT_NAMESPACE . 'postTouch':
131-
return $this->event->getSubject();
132-
case self::EVENT_NAMESPACE . 'postRename':
133-
case self::EVENT_NAMESPACE . 'postCopy':
134-
return $this->event->getSubject()[1];
135-
case MapperEvent::EVENT_ASSIGN:
136-
if (!$this->event instanceof MapperEvent || $this->event->getObjectType() !== 'files') {
137-
throw new NotFoundException();
138-
}
139-
$this->node = $this->root->getFirstNodeById((int)$this->event->getObjectId());
140-
if ($this->node !== null) {
141-
return $this->node;
142-
}
143-
break;
145+
146+
$this->node = $this->root->getFirstNodeById((int)$this->event->getObjectId());
147+
if ($this->node === null) {
148+
throw new NotFoundException();
144149
}
145-
throw new NotFoundException();
150+
151+
return $this->node;
146152
}
147153

148154
public function getDisplayText(int $verbosity = 0): string {
149155
try {
150156
$node = $this->getNode();
151-
} catch (NotFoundException $e) {
157+
} catch (NotFoundException) {
152158
return '';
153159
}
154160

@@ -158,21 +164,21 @@ public function getDisplayText(int $verbosity = 0): string {
158164
];
159165

160166
switch ($this->eventName) {
161-
case self::EVENT_NAMESPACE . 'postCreate':
167+
case NodeCreatedEvent::class:
162168
return $this->l10n->t('%s created %s', $options);
163-
case self::EVENT_NAMESPACE . 'postWrite':
169+
case NodeUpdatedEvent::class:
164170
return $this->l10n->t('%s modified %s', $options);
165-
case self::EVENT_NAMESPACE . 'postDelete':
171+
case NodeDeletedEvent::class:
166172
return $this->l10n->t('%s deleted %s', $options);
167-
case self::EVENT_NAMESPACE . 'postTouch':
173+
case NodeTouchedEvent::class:
168174
return $this->l10n->t('%s accessed %s', $options);
169-
case self::EVENT_NAMESPACE . 'postRename':
175+
case NodeRenamedEvent::class:
170176
return $this->l10n->t('%s renamed %s', $options);
171-
case self::EVENT_NAMESPACE . 'postCopy':
177+
case NodeCopiedEvent::class:
172178
return $this->l10n->t('%s copied %s', $options);
173-
case MapperEvent::EVENT_ASSIGN:
179+
case SingleTagAssignedEvent::class:
174180
$tagNames = [];
175-
if ($this->event instanceof MapperEvent) {
181+
if ($this->event instanceof SingleTagAssignedEvent) {
176182
$tagIDs = $this->event->getTags();
177183
$tagObjects = $this->tagManager->getTagsByIds($tagIDs);
178184
foreach ($tagObjects as $systemTag) {
@@ -201,9 +207,7 @@ public function getUrl(): string {
201207
}
202208
}
203209

204-
/**
205-
* @inheritDoc
206-
*/
210+
#[Override]
207211
public function exportContextIDs(): array {
208212
$nodeOwner = $this->getNode()->getOwner();
209213
$actingUserId = null;
@@ -215,14 +219,12 @@ public function exportContextIDs(): array {
215219
return [
216220
'eventName' => $this->eventName,
217221
'nodeId' => $this->getNode()->getId(),
218-
'nodeOwnerId' => $nodeOwner ? $nodeOwner->getUID() : null,
222+
'nodeOwnerId' => $nodeOwner?->getUID(),
219223
'actingUserId' => $actingUserId,
220224
];
221225
}
222226

223-
/**
224-
* @inheritDoc
225-
*/
227+
#[Override]
226228
public function importContextIDs(array $contextIDs): void {
227229
$this->eventName = $contextIDs['eventName'];
228230
if ($contextIDs['nodeOwnerId'] !== null) {
@@ -237,9 +239,7 @@ public function importContextIDs(array $contextIDs): void {
237239
}
238240
}
239241

240-
/**
241-
* @inheritDoc
242-
*/
242+
#[Override]
243243
public function getIconUrl(): string {
244244
return $this->getIcon();
245245
}
Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,94 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
/**
6+
* SPDX-FileCopyrightText: 2026 Nextcloud GmbH
7+
* SPDX-FileContributor: Carl Schwan
8+
* SPDX-License-Identifier: AGPL-3.0-or-later
9+
*/
10+
11+
namespace OCA\WorkflowEngine\Migration;
12+
13+
use OC\SystemTag\Events\SingleTagAssignedEvent;
14+
use OCP\Files\Events\Node\NodeCopiedEvent;
15+
use OCP\Files\Events\Node\NodeCreatedEvent;
16+
use OCP\Files\Events\Node\NodeDeletedEvent;
17+
use OCP\Files\Events\Node\NodeRenamedEvent;
18+
use OCP\Files\Events\Node\NodeTouchedEvent;
19+
use OCP\Files\Events\Node\NodeUpdatedEvent;
20+
use OCP\IDBConnection;
21+
use OCP\Migration\Attributes\ModifyColumn;
22+
use OCP\Migration\IOutput;
23+
use OCP\Migration\SimpleMigrationStep;
24+
use Override;
25+
26+
#[ModifyColumn(table: 'flow_operations', name: 'events', description: 'Use new event names')]
27+
class Version3400Date20260227000000 extends SimpleMigrationStep {
28+
public function __construct(
29+
private readonly IDBConnection $connection,
30+
) {
31+
}
32+
33+
#[Override]
34+
public function postSchemaChange(IOutput $output, \Closure $schemaClosure, array $options) {
35+
$qb = $this->connection->getQueryBuilder();
36+
$lastId = null;
37+
while (true) {
38+
$qb->select('*')
39+
->from('flow_operations');
40+
if ($lastId !== null) {
41+
$qb->andWhere($qb->expr()->gt('id', $qb->createNamedParameter($lastId)));
42+
}
43+
$qb->setMaxResults(1000);
44+
$newMapping = [];
45+
$result = $qb->executeQuery();
46+
while ($row = $result->fetchAssociative()) {
47+
$events = json_decode($row['events'], true);
48+
$newEvents = array_map(function (string $eventName): string {
49+
return match ($eventName) {
50+
'\OCP\Files::postCreate' => NodeCreatedEvent::class,
51+
'\OCP\Files::postUpdate' => NodeUpdatedEvent::class,
52+
'\OCP\Files::postRename' => NodeRenamedEvent::class,
53+
'\OCP\Files::postDelete' => NodeDeletedEvent::class,
54+
'\OCP\Files::postTouch' => NodeTouchedEvent::class,
55+
'\OCP\Files::postCopy' => NodeCopiedEvent::class,
56+
'OCP\SystemTag\ISystemTagObjectMapper::assignTags' => SingleTagAssignedEvent::class,
57+
};
58+
}, $events);
59+
60+
if ($newEvents !== $events) {
61+
$newMapping[$row['id']] = json_encode($newEvents);
62+
}
63+
}
64+
$result->closeCursor();
65+
66+
try {
67+
if ($newMapping !== []) {
68+
$this->connection->beginTransaction();
69+
}
70+
foreach ($newMapping as $id => $events) {
71+
$update = $this->connection->getQueryBuilder();
72+
$update->update('flow_operations')
73+
->set('events', $update->createNamedParameter($events))
74+
->where($qb->expr()->eq('id', $update->createNamedParameter($id)))
75+
->executeStatement();
76+
}
77+
if ($newMapping !== []) {
78+
$this->connection->commit();
79+
}
80+
} catch (\Exception $e) {
81+
$this->connection->rollback();
82+
throw $e;
83+
}
84+
85+
if ($row !== false) {
86+
$lastId = $row['id'];
87+
} else {
88+
break;
89+
}
90+
}
91+
92+
return $schemaClosure();
93+
}
94+
}

apps/workflowengine/tests/ManagerTest.php

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
use OCP\EventDispatcher\Event;
1616
use OCP\EventDispatcher\IEventDispatcher;
1717
use OCP\Files\Events\Node\NodeCreatedEvent;
18+
use OCP\Files\Events\Node\NodeDeletedEvent;
1819
use OCP\Files\IRootFolder;
1920
use OCP\Files\Mount\IMountManager;
2021
use OCP\ICache;
@@ -441,12 +442,12 @@ public function testUpdateOperation(): void {
441442
$check2 = ['class' => ICheck::class, 'operator' => 'eq', 'value' => 23456];
442443

443444
/** @noinspection PhpUnhandledExceptionInspection */
444-
$op = $this->manager->updateOperation($opId1, 'Test01a', [$check1, $check2], 'foohur', $adminScope, $entity, ['\OCP\Files::postDelete']);
445+
$op = $this->manager->updateOperation($opId1, 'Test01a', [$check1, $check2], 'foohur', $adminScope, $entity, [NodeDeletedEvent::class]);
445446
$this->assertSame('Test01a', $op['name']);
446447
$this->assertSame('foohur', $op['operation']);
447448

448449
/** @noinspection PhpUnhandledExceptionInspection */
449-
$op = $this->manager->updateOperation($opId2, 'Test02a', [$check1], 'barfoo', $userScope, $entity, ['\OCP\Files::postDelete']);
450+
$op = $this->manager->updateOperation($opId2, 'Test02a', [$check1], 'barfoo', $userScope, $entity, [NodeDeletedEvent::class]);
450451
$this->assertSame('Test02a', $op['name']);
451452
$this->assertSame('barfoo', $op['operation']);
452453

0 commit comments

Comments
 (0)