diff --git a/apps/workflowengine/lib/Controller/AWorkflowController.php b/apps/workflowengine/lib/Controller/AWorkflowController.php index 24f5732183..2e3186e380 100644 --- a/apps/workflowengine/lib/Controller/AWorkflowController.php +++ b/apps/workflowengine/lib/Controller/AWorkflowController.php @@ -99,11 +99,12 @@ abstract class AWorkflowController extends OCSController { string $name, array $checks, string $operation, + string $entity, array $events ): DataResponse { $context = $this->getScopeContext(); try { - $operation = $this->manager->addOperation($class, $name, $checks, $operation, $context, $events); + $operation = $this->manager->addOperation($class, $name, $checks, $operation, $context, $entity, $events); $operation = $this->manager->formatOperation($operation); return new DataResponse($operation); } catch (\UnexpectedValueException $e) { @@ -125,10 +126,12 @@ abstract class AWorkflowController extends OCSController { string $name, array $checks, string $operation, + string $entity, array $events ): DataResponse { try { - $operation = $this->manager->updateOperation($id, $name, $checks, $operation, $this->getScopeContext(), $events); + $context = $this->getScopeContext(); + $operation = $this->manager->updateOperation($id, $name, $checks, $operation, $context, $entity, $events); $operation = $this->manager->formatOperation($operation); return new DataResponse($operation); } catch (\UnexpectedValueException $e) { diff --git a/apps/workflowengine/lib/Manager.php b/apps/workflowengine/lib/Manager.php index d0376e703e..f19aa46d4a 100644 --- a/apps/workflowengine/lib/Manager.php +++ b/apps/workflowengine/lib/Manager.php @@ -36,6 +36,7 @@ use OCP\ILogger; use OCP\IServerContainer; use OCP\IUserSession; use OCP\WorkflowEngine\ICheck; +use OCP\WorkflowEngine\IComplexOperation; use OCP\WorkflowEngine\IEntity; use OCP\WorkflowEngine\IEntityAware; use OCP\WorkflowEngine\IEntityEvent; @@ -244,6 +245,7 @@ class Manager implements IManager, IEntityAware { string $name, array $checkIds, string $operation, + string $entity, array $events ): int { $query = $this->connection->getQueryBuilder(); @@ -253,6 +255,7 @@ class Manager implements IManager, IEntityAware { 'name' => $query->createNamedParameter($name), 'checks' => $query->createNamedParameter(json_encode(array_unique($checkIds))), 'operation' => $query->createNamedParameter($operation), + 'entity' => $query->createNamedParameter($entity), 'events' => $query->createNamedParameter(json_encode($events)) ]); $query->execute(); @@ -275,9 +278,10 @@ class Manager implements IManager, IEntityAware { array $checks, string $operation, ScopeContext $scope, + string $entity, array $events ) { - $this->validateOperation($class, $name, $checks, $operation, $events); + $this->validateOperation($class, $name, $checks, $operation, $entity, $events); $this->connection->beginTransaction(); @@ -287,7 +291,7 @@ class Manager implements IManager, IEntityAware { $checkIds[] = $this->addCheck($check['class'], $check['operator'], $check['value']); } - $id = $this->insertOperation($class, $name, $checkIds, $operation, $events); + $id = $this->insertOperation($class, $name, $checkIds, $operation, $entity, $events); $this->addScope($id, $scope); $this->connection->commit(); @@ -342,13 +346,14 @@ class Manager implements IManager, IEntityAware { array $checks, string $operation, ScopeContext $scopeContext, + string $entity, array $events ): array { if(!$this->canModify($id, $scopeContext)) { throw new \DomainException('Target operation not within scope'); }; $row = $this->getOperation($id); - $this->validateOperation($row['class'], $name, $checks, $operation, $events); + $this->validateOperation($row['class'], $name, $checks, $operation, $entity, $events); $checkIds = []; try { @@ -362,6 +367,7 @@ class Manager implements IManager, IEntityAware { ->set('name', $query->createNamedParameter($name)) ->set('checks', $query->createNamedParameter(json_encode(array_unique($checkIds)))) ->set('operation', $query->createNamedParameter($operation)) + ->set('entity', $query->createNamedParameter($entity)) ->set('events', $query->createNamedParameter(json_encode($events))) ->where($query->expr()->eq('id', $query->createNamedParameter($id))); $query->execute(); @@ -411,27 +417,34 @@ class Manager implements IManager, IEntityAware { return $result; } - protected function validateEvents($events) { - foreach ($events as $entity => $eventNames) { - try { - /** @var IEntity $instance */ - $instance = $this->container->query($entity); - } catch (QueryException $e) { - throw new \UnexpectedValueException($this->l->t('Entity %s does not exist', [$entity])); - } + protected function validateEvents(string $entity, array $events, IOperation $operation) { + try { + /** @var IEntity $instance */ + $instance = $this->container->query($entity); + } catch (QueryException $e) { + throw new \UnexpectedValueException($this->l->t('Entity %s does not exist', [$entity])); + } - if(!$instance instanceof IEntity) { - throw new \UnexpectedValueException($this->l->t('Entity %s is invalid', [$entity])); - } + if(!$instance instanceof IEntity) { + throw new \UnexpectedValueException($this->l->t('Entity %s is invalid', [$entity])); + } - $availableEvents = array_reduce($instance->getEvents(), function(array $carry, IEntityEvent $event) { - $carry[] = $event->getEventName(); - }, []); - foreach($eventNames as $event) { - if(!in_array($event, $availableEvents, true)) { - throw new \UnexpectedValueException($this->l->t('Entity %s has no event %s', [$entity, $event])); - } + if(empty($events)) { + if(!$operation instanceof IComplexOperation) { + throw new \UnexpectedValueException($this->l->t('No events are chosen.')); } + return; + } + + $availableEvents = []; + foreach ($instance->getEvents() as $event) { + /** @var IEntityEvent $event */ + $availableEvents[] = $event->getEventName(); + } + + $diff = array_diff($events, $availableEvents); + if(!empty($diff)) { + throw new \UnexpectedValueException($this->l->t('Entity %s has no event %s', [$entity, array_shift($diff)])); } } @@ -442,7 +455,7 @@ class Manager implements IManager, IEntityAware { * @param string $operation * @throws \UnexpectedValueException */ - protected function validateOperation($class, $name, array $checks, $operation, array $events) { + protected function validateOperation($class, $name, array $checks, $operation, string $entity, array $events) { try { /** @var IOperation $instance */ $instance = $this->container->query($class); @@ -454,7 +467,7 @@ class Manager implements IManager, IEntityAware { throw new \UnexpectedValueException($this->l->t('Operation %s is invalid', [$class])); } - $this->validateEvents($events); + $this->validateEvents($entity, $events, $instance); $instance->validateOperation($name, $checks, $operation); diff --git a/apps/workflowengine/lib/Migration/PopulateNewlyIntroducedDatabaseFields.php b/apps/workflowengine/lib/Migration/PopulateNewlyIntroducedDatabaseFields.php index 43595d1c7c..8b3f3d5a9c 100644 --- a/apps/workflowengine/lib/Migration/PopulateNewlyIntroducedDatabaseFields.php +++ b/apps/workflowengine/lib/Migration/PopulateNewlyIntroducedDatabaseFields.php @@ -25,6 +25,7 @@ declare(strict_types=1); namespace OCA\WorkflowEngine\Migration; use Doctrine\DBAL\Driver\Statement; +use OCA\WorkflowEngine\Entity\File; use OCP\IDBConnection; use OCP\Migration\IOutput; use OCP\Migration\IRepairStep; @@ -49,6 +50,18 @@ class PopulateNewlyIntroducedDatabaseFields implements IRepairStep { $this->populateScopeTable($result); $result->closeCursor(); + + $this->populateEntityCol(); + } + + protected function populateEntityCol() { + $qb = $this->dbc->getQueryBuilder(); + + $qb->update('flow_operations') + ->set('entity', File::class) + ->where($qb->expr()->emptyString('entity')) + ->execute(); + } protected function populateScopeTable(Statement $ids): void { diff --git a/apps/workflowengine/lib/Migration/Version2019Date20190808074233.php b/apps/workflowengine/lib/Migration/Version2019Date20190808074233.php index 2b9a8aa17c..ffea3c88d0 100644 --- a/apps/workflowengine/lib/Migration/Version2019Date20190808074233.php +++ b/apps/workflowengine/lib/Migration/Version2019Date20190808074233.php @@ -70,12 +70,12 @@ class Version2019Date20190808074233 extends SimpleMigrationStep { $table->addColumn('operation', Type::TEXT, [ 'notnull' => false, ]); - $this->addEventsColumn($table); + $this->addEntityColumns($table); $table->setPrimaryKey(['id']); } else { $table = $schema->getTable('flow_operations'); - if(!$table->hasColumn('events')) { - $this->addEventsColumn($table); + if(!$table->hasColumn('entity')) { + $this->addEntityColumns($table); } } @@ -105,7 +105,11 @@ class Version2019Date20190808074233 extends SimpleMigrationStep { return $schema; } - protected function addEventsColumn(Table $table) { + protected function addEntityColumns(Table $table) { + $table->addColumn('entity', Type::STRING, [ + 'notnull' => true, + 'length' => 256, + ]); $table->addColumn('events', Type::TEXT, [ 'notnull' => true, 'default' => '[]', diff --git a/apps/workflowengine/tests/ManagerTest.php b/apps/workflowengine/tests/ManagerTest.php index a2d30944f7..3a0cd18a94 100644 --- a/apps/workflowengine/tests/ManagerTest.php +++ b/apps/workflowengine/tests/ManagerTest.php @@ -22,6 +22,7 @@ namespace OCA\WorkflowEngine\Tests; +use OC\L10N\L10N; use OCA\WorkflowEngine\Entity\File; use OCA\WorkflowEngine\Helper\ScopeContext; use OCA\WorkflowEngine\Manager; @@ -29,6 +30,7 @@ use OCP\IDBConnection; use OCP\IL10N; use OCP\ILogger; use OCP\IServerContainer; +use OCP\IURLGenerator; use OCP\IUserSession; use OCP\WorkflowEngine\ICheck; use OCP\WorkflowEngine\IEntity; @@ -141,24 +143,25 @@ class ManagerTest extends TestCase { public function testScope() { $adminScope = $this->buildScope(); $userScope = $this->buildScope('jackie'); + $entity = File::class; $opId1 = $this->invokePrivate( $this->manager, 'insertOperation', - ['OCA\WFE\TestOp', 'Test01', [11, 22], 'foo'] + ['OCA\WFE\TestOp', 'Test01', [11, 22], 'foo', $entity, []] ); $this->invokePrivate($this->manager, 'addScope', [$opId1, $adminScope]); $opId2 = $this->invokePrivate( $this->manager, 'insertOperation', - ['OCA\WFE\TestOp', 'Test02', [33, 22], 'bar'] + ['OCA\WFE\TestOp', 'Test02', [33, 22], 'bar', $entity, []] ); $this->invokePrivate($this->manager, 'addScope', [$opId2, $userScope]); $opId3 = $this->invokePrivate( $this->manager, 'insertOperation', - ['OCA\WFE\TestOp', 'Test03', [11, 44], 'foobar'] + ['OCA\WFE\TestOp', 'Test03', [11, 44], 'foobar', $entity, []] ); $this->invokePrivate($this->manager, 'addScope', [$opId3, $userScope]); @@ -174,24 +177,25 @@ class ManagerTest extends TestCase { public function testGetAllOperations() { $adminScope = $this->buildScope(); $userScope = $this->buildScope('jackie'); + $entity = File::class; $opId1 = $this->invokePrivate( $this->manager, 'insertOperation', - ['OCA\WFE\TestAdminOp', 'Test01', [11, 22], 'foo'] + ['OCA\WFE\TestAdminOp', 'Test01', [11, 22], 'foo', $entity, []] ); $this->invokePrivate($this->manager, 'addScope', [$opId1, $adminScope]); $opId2 = $this->invokePrivate( $this->manager, 'insertOperation', - ['OCA\WFE\TestUserOp', 'Test02', [33, 22], 'bar'] + ['OCA\WFE\TestUserOp', 'Test02', [33, 22], 'bar', $entity, []] ); $this->invokePrivate($this->manager, 'addScope', [$opId2, $userScope]); $opId3 = $this->invokePrivate( $this->manager, 'insertOperation', - ['OCA\WFE\TestUserOp', 'Test03', [11, 44], 'foobar'] + ['OCA\WFE\TestUserOp', 'Test03', [11, 44], 'foobar', $entity, []] ); $this->invokePrivate($this->manager, 'addScope', [$opId3, $userScope]); @@ -211,36 +215,37 @@ class ManagerTest extends TestCase { public function testGetOperations() { $adminScope = $this->buildScope(); $userScope = $this->buildScope('jackie'); + $entity = File::class; $opId1 = $this->invokePrivate( $this->manager, 'insertOperation', - ['OCA\WFE\TestOp', 'Test01', [11, 22], 'foo'] + ['OCA\WFE\TestOp', 'Test01', [11, 22], 'foo', $entity, []] ); $this->invokePrivate($this->manager, 'addScope', [$opId1, $adminScope]); $opId4 = $this->invokePrivate( $this->manager, 'insertOperation', - ['OCA\WFE\OtherTestOp', 'Test04', [5], 'foo'] + ['OCA\WFE\OtherTestOp', 'Test04', [5], 'foo', $entity, []] ); $this->invokePrivate($this->manager, 'addScope', [$opId4, $adminScope]); $opId2 = $this->invokePrivate( $this->manager, 'insertOperation', - ['OCA\WFE\TestOp', 'Test02', [33, 22], 'bar'] + ['OCA\WFE\TestOp', 'Test02', [33, 22], 'bar', $entity, []] ); $this->invokePrivate($this->manager, 'addScope', [$opId2, $userScope]); $opId3 = $this->invokePrivate( $this->manager, 'insertOperation', - ['OCA\WFE\TestOp', 'Test03', [11, 44], 'foobar'] + ['OCA\WFE\TestOp', 'Test03', [11, 44], 'foobar', $entity, []] ); $this->invokePrivate($this->manager, 'addScope', [$opId3, $userScope]); $opId5 = $this->invokePrivate( $this->manager, 'insertOperation', - ['OCA\WFE\OtherTestOp', 'Test05', [5], 'foobar'] + ['OCA\WFE\OtherTestOp', 'Test05', [5], 'foobar', $entity, []] ); $this->invokePrivate($this->manager, 'addScope', [$opId5, $userScope]); @@ -262,12 +267,18 @@ class ManagerTest extends TestCase { public function testUpdateOperation() { $adminScope = $this->buildScope(); $userScope = $this->buildScope('jackie'); + $entity = File::class; $this->container->expects($this->any()) ->method('query') ->willReturnCallback(function ($class) { if(substr($class, -2) === 'Op') { return $this->createMock(IOperation::class); + } else if($class === File::class) { + return $this->getMockBuilder(File::class) + ->setConstructorArgs([$this->createMock(L10N::class), $this->createMock(IURLGenerator::class)]) + ->setMethodsExcept(['getEvents']) + ->getMock(); } return $this->createMock(ICheck::class); }); @@ -275,14 +286,14 @@ class ManagerTest extends TestCase { $opId1 = $this->invokePrivate( $this->manager, 'insertOperation', - ['OCA\WFE\TestAdminOp', 'Test01', [11, 22], 'foo'] + ['OCA\WFE\TestAdminOp', 'Test01', [11, 22], 'foo', $entity, []] ); $this->invokePrivate($this->manager, 'addScope', [$opId1, $adminScope]); $opId2 = $this->invokePrivate( $this->manager, 'insertOperation', - ['OCA\WFE\TestUserOp', 'Test02', [33, 22], 'bar'] + ['OCA\WFE\TestUserOp', 'Test02', [33, 22], 'bar', $entity, []] ); $this->invokePrivate($this->manager, 'addScope', [$opId2, $userScope]); @@ -290,19 +301,19 @@ class ManagerTest extends TestCase { $check2 = ['class' => 'OCA\WFE\C33', 'operator' => 'eq', 'value' => 23456]; /** @noinspection PhpUnhandledExceptionInspection */ - $op = $this->manager->updateOperation($opId1, 'Test01a', [$check1, $check2], 'foohur', $adminScope, []); + $op = $this->manager->updateOperation($opId1, 'Test01a', [$check1, $check2], 'foohur', $adminScope, $entity, ['postDelete']); $this->assertSame('Test01a', $op['name']); $this->assertSame('foohur', $op['operation']); /** @noinspection PhpUnhandledExceptionInspection */ - $op = $this->manager->updateOperation($opId2, 'Test02a', [$check1], 'barfoo', $userScope, []); + $op = $this->manager->updateOperation($opId2, 'Test02a', [$check1], 'barfoo', $userScope, $entity, ['postDelete']); $this->assertSame('Test02a', $op['name']); $this->assertSame('barfoo', $op['operation']); foreach([[$adminScope, $opId2], [$userScope, $opId1]] as $run) { try { /** @noinspection PhpUnhandledExceptionInspection */ - $this->manager->updateOperation($run[1], 'Evil', [$check2], 'hackx0r', $run[0], []); + $this->manager->updateOperation($run[1], 'Evil', [$check2], 'hackx0r', $run[0], $entity, []); $this->assertTrue(false, 'DomainException not thrown'); } catch (\DomainException $e) { $this->assertTrue(true); @@ -313,18 +324,19 @@ class ManagerTest extends TestCase { public function testDeleteOperation() { $adminScope = $this->buildScope(); $userScope = $this->buildScope('jackie'); + $entity = File::class; $opId1 = $this->invokePrivate( $this->manager, 'insertOperation', - ['OCA\WFE\TestAdminOp', 'Test01', [11, 22], 'foo'] + ['OCA\WFE\TestAdminOp', 'Test01', [11, 22], 'foo', $entity, []] ); $this->invokePrivate($this->manager, 'addScope', [$opId1, $adminScope]); $opId2 = $this->invokePrivate( $this->manager, 'insertOperation', - ['OCA\WFE\TestUserOp', 'Test02', [33, 22], 'bar'] + ['OCA\WFE\TestUserOp', 'Test02', [33, 22], 'bar', $entity, []] ); $this->invokePrivate($this->manager, 'addScope', [$opId2, $userScope]);