Merge pull request #36568 from nextcloud/perf/flow-events

perf(workflowengine): Cache query that is performed on every request
nextcloud · Feb 13, 2023 · 8f0849a · 8f0849a
2 parents c3d0009 + aebf654
commit 8f0849a
Show file tree

Hide file tree

Showing 2 changed files with 72 additions and 4 deletions.
diff --git a/apps/workflowengine/lib/Manager.php b/apps/workflowengine/lib/Manager.php
@@ -49,6 +49,7 @@
 use OCP\DB\QueryBuilder\IQueryBuilder;
 use OCP\EventDispatcher\IEventDispatcher;
 use OCP\Files\Storage\IStorage;
+use OCP\ICacheFactory;
 use OCP\IConfig;
 use OCP\IDBConnection;
 use OCP\IL10N;
@@ -69,7 +70,6 @@
 use Symfony\Component\EventDispatcher\GenericEvent;
 
 class Manager implements IManager {
-
 	/** @var IStorage */
 	protected $storage;
 
@@ -120,6 +120,7 @@ class Manager implements IManager {
 
 	/** @var IConfig */
 	private $config;
+	private ICacheFactory $cacheFactory;
 
 	public function __construct(
 		IDBConnection $connection,
@@ -129,7 +130,8 @@ public function __construct(
 		ILogger $logger,
 		IUserSession $session,
 		IEventDispatcher $dispatcher,
-		IConfig $config
+		IConfig $config,
+		ICacheFactory $cacheFactory,
 	) {
 		$this->connection = $connection;
 		$this->container = $container;
@@ -140,6 +142,7 @@ public function __construct(
 		$this->session = $session;
 		$this->dispatcher = $dispatcher;
 		$this->config = $config;
+		$this->cacheFactory = $cacheFactory;
 	}
 
 	public function getRuleMatcher(): IRuleMatcher {
@@ -153,6 +156,12 @@ public function getRuleMatcher(): IRuleMatcher {
 	}
 
 	public function getAllConfiguredEvents() {
+		$cache = $this->cacheFactory->createDistributed('flow');
+		$cached = $cache->get('events');
+		if ($cached !== null) {
+			return $cached;
+		}
+
 		$query = $this->connection->getQueryBuilder();
 
 		$query->select('class', 'entity')
@@ -176,6 +185,8 @@ public function getAllConfiguredEvents() {
 		}
 		$result->closeCursor();
 
+		$cache->set('events', $operations, 3600);
+
 		return $operations;
 	}
 
@@ -289,6 +300,8 @@ protected function insertOperation(
 			]);
 		$query->execute();
 
+		$this->cacheFactory->createDistributed('flow')->remove('events');
+
 		return $query->getLastInsertId();
 	}
 
@@ -407,6 +420,7 @@ public function updateOperation(
 			throw $e;
 		}
 		unset($this->operations[$scopeContext->getHash()]);
+		$this->cacheFactory->createDistributed('flow')->remove('events');
 
 		return $this->getOperation($id);
 	}
@@ -444,6 +458,8 @@ public function deleteOperation($id, ScopeContext $scopeContext) {
 			unset($this->operations[$scopeContext->getHash()]);
 		}
 
+		$this->cacheFactory->createDistributed('flow')->remove('events');
+
 		return $result;
 	}
 

diff --git a/apps/workflowengine/tests/ManagerTest.php b/apps/workflowengine/tests/ManagerTest.php
@@ -31,7 +31,10 @@
 use OCA\WorkflowEngine\Helper\ScopeContext;
 use OCA\WorkflowEngine\Manager;
 use OCP\EventDispatcher\IEventDispatcher;
+use OCP\Files\Events\Node\NodeCreatedEvent;
 use OCP\Files\IRootFolder;
+use OCP\ICache;
+use OCP\ICacheFactory;
 use OCP\IConfig;
 use OCP\IDBConnection;
 use OCP\IL10N;
@@ -57,7 +60,6 @@
  * @group DB
  */
 class ManagerTest extends TestCase {
-
 	/** @var Manager */
 	protected $manager;
 	/** @var MockObject|IDBConnection */
@@ -76,6 +78,8 @@ class ManagerTest extends TestCase {
 	protected $dispatcher;
 	/** @var MockObject|IConfig */
 	protected $config;
+	/** @var MockObject|ICacheFactory  */
+	protected $cacheFactory;
 
 	protected function setUp(): void {
 		parent::setUp();
@@ -94,6 +98,7 @@ protected function setUp(): void {
 		$this->session = $this->createMock(IUserSession::class);
 		$this->dispatcher = $this->createMock(IEventDispatcher::class);
 		$this->config = $this->createMock(IConfig::class);
+		$this->cacheFactory = $this->createMock(ICacheFactory::class);
 
 		$this->manager = new Manager(
 			\OC::$server->getDatabaseConnection(),
@@ -103,7 +108,8 @@ protected function setUp(): void {
 			$this->logger,
 			$this->session,
 			$this->dispatcher,
-			$this->config
+			$this->config,
+			$this->cacheFactory
 		);
 		$this->clearTables();
 	}
@@ -283,11 +289,51 @@ public function testGetOperations() {
 		});
 	}
 
+	public function testGetAllConfiguredEvents() {
+		$adminScope = $this->buildScope();
+		$userScope = $this->buildScope('jackie');
+		$entity = File::class;
+
+		$opId5 = $this->invokePrivate(
+			$this->manager,
+			'insertOperation',
+			['OCA\WFE\OtherTestOp', 'Test04', [], 'foo', $entity, [NodeCreatedEvent::class]]
+		);
+		$this->invokePrivate($this->manager, 'addScope', [$opId5, $userScope]);
+
+		$allOperations = null;
+
+		$cache = $this->createMock(ICache::class);
+		$cache
+			->method('get')
+			->willReturnCallback(function () use (&$allOperations) {
+				if ($allOperations) {
+					return $allOperations;
+				}
+
+				return null;
+			});
+
+		$this->cacheFactory->method('createDistributed')->willReturn($cache);
+		$allOperations = $this->manager->getAllConfiguredEvents();
+		$this->assertCount(1, $allOperations);
+
+		$allOperationsCached = $this->manager->getAllConfiguredEvents();
+		$this->assertCount(1, $allOperationsCached);
+		$this->assertEquals($allOperationsCached, $allOperations);
+	}
+
 	public function testUpdateOperation() {
 		$adminScope = $this->buildScope();
 		$userScope = $this->buildScope('jackie');
 		$entity = File::class;
 
+		$cache = $this->createMock(ICache::class);
+		$cache->expects($this->exactly(4))
+			->method('remove')
+			->with('events');
+		$this->cacheFactory->method('createDistributed')->willReturn($cache);
+
 		$this->container->expects($this->any())
 			->method('query')
 			->willReturnCallback(function ($class) {
@@ -354,6 +400,12 @@ public function testDeleteOperation() {
 		$userScope = $this->buildScope('jackie');
 		$entity = File::class;
 
+		$cache = $this->createMock(ICache::class);
+		$cache->expects($this->exactly(4))
+			->method('remove')
+			->with('events');
+		$this->cacheFactory->method('createDistributed')->willReturn($cache);
+
 		$opId1 = $this->invokePrivate(
 			$this->manager,
 			'insertOperation',