split events DB field into entity and events, adjust biz logic

Signed-off-by: Arthur Schiwon <blizzz@arthur-schiwon.de>

5 files changed, 92 insertions, 47 deletions

diff --git a/apps/workflowengine/lib/Controller/AWorkflowController.php b/apps/workflowengine/lib/Controller/AWorkflowController.php
index 24f57321834..2e3186e380d 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 d0376e703e0..f19aa46d4ab 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 43595d1c7cf..8b3f3d5a9c3 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 2b9a8aa17cd..ffea3c88d01 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 a2d30944f78..3a0cd18a946 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]);