From aebf6542140f593beb03a5c897f74d3ab057b362 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Julius=20H=C3=A4rtl?= Date: Mon, 6 Feb 2023 20:08:37 +0100 Subject: [PATCH] perf(workflowengine): Cache query that is performed on every request MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit Signed-off-by: Julius Härtl --- apps/workflowengine/lib/Manager.php | 20 +++++++- apps/workflowengine/tests/ManagerTest.php | 56 ++++++++++++++++++++++- 2 files changed, 72 insertions(+), 4 deletions(-) diff --git a/apps/workflowengine/lib/Manager.php b/apps/workflowengine/lib/Manager.php index 659fd2421c1..417e1a1eba6 100644 --- a/apps/workflowengine/lib/Manager.php +++ b/apps/workflowengine/lib/Manager.php @@ -49,6 +49,7 @@ use OCP\AppFramework\QueryException; 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\EventDispatcherInterface as LegacyDispatch 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 @@ class Manager implements IManager { ILogger $logger, IUserSession $session, IEventDispatcher $dispatcher, - IConfig $config + IConfig $config, + ICacheFactory $cacheFactory, ) { $this->connection = $connection; $this->container = $container; @@ -140,6 +142,7 @@ class Manager implements IManager { $this->session = $session; $this->dispatcher = $dispatcher; $this->config = $config; + $this->cacheFactory = $cacheFactory; } public function getRuleMatcher(): IRuleMatcher { @@ -153,6 +156,12 @@ class Manager implements IManager { } 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 @@ class Manager implements IManager { } $result->closeCursor(); + $cache->set('events', $operations, 3600); + return $operations; } @@ -289,6 +300,8 @@ class Manager implements IManager { ]); $query->execute(); + $this->cacheFactory->createDistributed('flow')->remove('events'); + return $query->getLastInsertId(); } @@ -407,6 +420,7 @@ class Manager implements IManager { throw $e; } unset($this->operations[$scopeContext->getHash()]); + $this->cacheFactory->createDistributed('flow')->remove('events'); return $this->getOperation($id); } @@ -444,6 +458,8 @@ class Manager implements IManager { 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 index 612495a5b6d..d3678b66bee 100644 --- a/apps/workflowengine/tests/ManagerTest.php +++ b/apps/workflowengine/tests/ManagerTest.php @@ -31,7 +31,10 @@ use OCA\WorkflowEngine\Entity\File; 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 @@ use Test\TestCase; * @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 @@ class ManagerTest extends TestCase { $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 @@ class ManagerTest extends TestCase { $this->logger, $this->session, $this->dispatcher, - $this->config + $this->config, + $this->cacheFactory ); $this->clearTables(); } @@ -283,11 +289,51 @@ class ManagerTest extends TestCase { }); } + 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 @@ class ManagerTest extends TestCase { $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', -- 2.39.5