From 65613e29d06a35ab1193993a8f59ad0a06b04653 Mon Sep 17 00:00:00 2001 From: Andrei Shapiro Date: Mon, 9 Oct 2023 16:07:12 +0300 Subject: [PATCH 01/11] chore: rollback to handle only single resource --- .../event/DataAccessControlChangedEvent.php | 35 +++---------------- .../DataAccessControlChangedListener.php | 20 +++-------- 2 files changed, 8 insertions(+), 47 deletions(-) diff --git a/models/classes/event/DataAccessControlChangedEvent.php b/models/classes/event/DataAccessControlChangedEvent.php index fbc79957e4..b8efbd6bc7 100644 --- a/models/classes/event/DataAccessControlChangedEvent.php +++ b/models/classes/event/DataAccessControlChangedEvent.php @@ -26,35 +26,18 @@ class DataAccessControlChangedEvent implements Event { - /** @var string */ - private $resourceId; - - /** @var ?string */ - private $rootResourceId; - - /** @var array */ - private $addRemove; - - /** - * @deprecated Use $applyToNestedResources cause processing recursively causes performance issues - * @var bool - */ - private $isRecursive; - - private bool $applyToNestedResources; + private string $resourceId; + private array $addRemove; + private bool $isRecursive; public function __construct( string $resourceId, array $addRemove, - bool $isRecursive = false, - bool $applyToNestedResources = false, - string $rootResourceId = null + bool $isRecursive = false ) { $this->resourceId = $resourceId; $this->addRemove = $addRemove; $this->isRecursive = $isRecursive; - $this->applyToNestedResources = $applyToNestedResources; - $this->rootResourceId = $rootResourceId; } public function getName(): string @@ -67,11 +50,6 @@ public function getResourceId(): string return $this->resourceId; } - public function getRootResourceId(): ?string - { - return $this->rootResourceId; - } - public function getOperations(string $operation): array { return array_keys($this->addRemove[$operation] ?? []); @@ -84,9 +62,4 @@ public function isRecursive(): bool { return $this->isRecursive; } - - public function applyToNestedResources(): bool - { - return $this->applyToNestedResources; - } } diff --git a/models/classes/listener/DataAccessControlChangedListener.php b/models/classes/listener/DataAccessControlChangedListener.php index c16795c567..51f5519a69 100644 --- a/models/classes/listener/DataAccessControlChangedListener.php +++ b/models/classes/listener/DataAccessControlChangedListener.php @@ -49,19 +49,12 @@ public function handleEvent(DataAccessControlChangedEvent $event): void /** @noinspection PhpUnhandledExceptionInspection */ $resource = $this->getResource($event->getResourceId()); - if ($event->getResourceId() !== $event->getRootResourceId()) { + if ($resource->isClass() && !$event->isRecursive()) { return; } - $this->getLogger()->debug( - sprintf( - 'Dispatching UpdateDataAccessControlInIndex for root resource %s [%s]', - $resource->getLabel(), - $resource->getLabel() - ) - ); - - $this->getQueueDispatcher()->createTask( + $queueDispatcher = $this->getServiceLocator()->get(QueueDispatcherInterface::SERVICE_ID); + $queueDispatcher->createTask( new UpdateDataAccessControlInIndex(), [ $resource->getUri(), @@ -70,9 +63,4 @@ public function handleEvent(DataAccessControlChangedEvent $event): void $taskMessage ); } - - private function getQueueDispatcher(): QueueDispatcherInterface - { - return $this->getServiceLocator()->get(QueueDispatcherInterface::SERVICE_ID); - } -} +} \ No newline at end of file From a60e535d366c75455f62e3798a3d14cbe7ae8cea Mon Sep 17 00:00:00 2001 From: Gabriel Felipe Soares Date: Wed, 18 Oct 2023 11:51:04 +0200 Subject: [PATCH 02/11] chore: add method to remove queues --- models/classes/taskQueue/QueueDispatcher.php | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/models/classes/taskQueue/QueueDispatcher.php b/models/classes/taskQueue/QueueDispatcher.php index f580d878d0..a35c056919 100644 --- a/models/classes/taskQueue/QueueDispatcher.php +++ b/models/classes/taskQueue/QueueDispatcher.php @@ -160,6 +160,21 @@ public function addQueue(QueueInterface $queue) return $this; } + public function removeQueue(string $queueName): self + { + $queues = $this->getQueues(); + + foreach ($queues as $key => $queue) { + if ($queue->getName() === $queueName) { + unset($queues[$key]); + } + } + + $this->setQueues(array_values($queues)); + + return $this; + } + /** * @inheritdoc */ From 4b3f8758cc916a993a9c97b9900bd0c06a874289 Mon Sep 17 00:00:00 2001 From: Gabriel Felipe Soares Date: Fri, 20 Oct 2023 12:02:28 +0200 Subject: [PATCH 03/11] chore: remove deprecation --- models/classes/event/DataAccessControlChangedEvent.php | 3 --- 1 file changed, 3 deletions(-) diff --git a/models/classes/event/DataAccessControlChangedEvent.php b/models/classes/event/DataAccessControlChangedEvent.php index b8efbd6bc7..b3b3f313ce 100644 --- a/models/classes/event/DataAccessControlChangedEvent.php +++ b/models/classes/event/DataAccessControlChangedEvent.php @@ -55,9 +55,6 @@ public function getOperations(string $operation): array return array_keys($this->addRemove[$operation] ?? []); } - /** - * @deprecated Use applyToNestedResources because processing recursively causes performance issues - */ public function isRecursive(): bool { return $this->isRecursive; From a20c11839e841b6e204ffcb4f72fbffcd3a608ff Mon Sep 17 00:00:00 2001 From: Gabriel Felipe Soares Date: Mon, 23 Oct 2023 14:25:01 +0200 Subject: [PATCH 04/11] chore: add removeQueue method --- models/classes/taskQueue/QueueDispatcher.php | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/models/classes/taskQueue/QueueDispatcher.php b/models/classes/taskQueue/QueueDispatcher.php index f580d878d0..a35c056919 100644 --- a/models/classes/taskQueue/QueueDispatcher.php +++ b/models/classes/taskQueue/QueueDispatcher.php @@ -160,6 +160,21 @@ public function addQueue(QueueInterface $queue) return $this; } + public function removeQueue(string $queueName): self + { + $queues = $this->getQueues(); + + foreach ($queues as $key => $queue) { + if ($queue->getName() === $queueName) { + unset($queues[$key]); + } + } + + $this->setQueues(array_values($queues)); + + return $this; + } + /** * @inheritdoc */ From fed2aa241730ee5e7cba697e019c75243374f3bb Mon Sep 17 00:00:00 2001 From: Gabriel Felipe Soares Date: Mon, 23 Oct 2023 16:29:04 +0200 Subject: [PATCH 05/11] chore: get default ServiceManger in case service locator is not set --- models/classes/taskQueue/Queue.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/models/classes/taskQueue/Queue.php b/models/classes/taskQueue/Queue.php index 2feeb3020e..3599e55aa8 100644 --- a/models/classes/taskQueue/Queue.php +++ b/models/classes/taskQueue/Queue.php @@ -23,6 +23,7 @@ use oat\oatbox\log\LoggerAwareTrait; use oat\oatbox\mutex\LockTrait; +use oat\oatbox\service\ServiceManager; use oat\tao\model\taskQueue\Queue\Broker\QueueBrokerInterface; use oat\tao\model\taskQueue\Queue\Broker\SyncQueueBrokerInterface; use oat\tao\model\taskQueue\Task\TaskInterface; @@ -142,7 +143,7 @@ public function setBroker(QueueBrokerInterface $broker) public function getBroker(): QueueBrokerInterface { - $this->broker->setServiceLocator($this->getServiceLocator()); + $this->broker->setServiceLocator($this->getServiceLocator() ?? ServiceManager::getServiceManager()); return $this->broker; } From c98cd3bd813cd4d17d125f6859a5950032f66e4b Mon Sep 17 00:00:00 2001 From: Andrei Shapiro Date: Thu, 2 Nov 2023 14:11:43 +0300 Subject: [PATCH 06/11] chore: add unit test --- .../DataAccessControlChangedListenerTest.php | 71 +++++++------------ .../models/classes/Csv/Service/ReaderTest.php | 18 +---- 2 files changed, 25 insertions(+), 64 deletions(-) diff --git a/test/unit/model/listener/DataAccessControlChangedListenerTest.php b/test/unit/model/listener/DataAccessControlChangedListenerTest.php index fde09fc876..c1c70a9324 100644 --- a/test/unit/model/listener/DataAccessControlChangedListenerTest.php +++ b/test/unit/model/listener/DataAccessControlChangedListenerTest.php @@ -15,25 +15,27 @@ * along with this program; if not, write to the Free Software * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. * - * Copyright (c) 2020-2023 (original work) Open Assessment Technologies SA; - * + * Copyright (c) 2023 (original work) Open Assessment Technologies SA. */ declare(strict_types=1); use oat\generis\model\data\Ontology; use oat\generis\test\MockObject; -use oat\generis\test\TestCase; +use oat\generis\test\ServiceManagerMockTrait; use oat\oatbox\log\LoggerService; use oat\tao\model\AdvancedSearch\AdvancedSearchChecker; use oat\tao\model\event\DataAccessControlChangedEvent; use oat\tao\model\listener\DataAccessControlChangedListener; use oat\tao\model\search\tasks\UpdateDataAccessControlInIndex; use oat\tao\model\taskQueue\QueueDispatcherInterface; +use PHPUnit\Framework\TestCase; use Psr\Log\LoggerInterface; class DataAccessControlChangedListenerTest extends TestCase { + use ServiceManagerMockTrait; + /** @var QueueDispatcherInterface|MockObject */ private $queueDispatcher; @@ -59,7 +61,7 @@ protected function setUp(): void $this->advancedSearchChecker = $this->createMock(AdvancedSearchChecker::class); $this->sut->setServiceLocator( - $this->getServiceLocatorMock( + $this->getServiceManagerMock( [ QueueDispatcherInterface::SERVICE_ID => $this->queueDispatcher, LoggerService::SERVICE_ID => $this->logger, @@ -71,45 +73,26 @@ protected function setUp(): void } /** - * @dataProvider handleEventSuccessfulCasesDataProvider + * @dataProvider provideSuccessfulCases */ - public function testHandleEventShouldCreateTaskSuccessfully(bool $isRecursive): void + public function testHandleEventShouldCreateTaskSuccessfully(bool $isRecursive, bool $isClass): void { $documentUri = 'https://tao.docker.localhost/ontologies/tao.rdf#i5ef45f413088c8e7901a84708e84ec'; - $resource = $this->createMock(core_kernel_classes_Resource::class); + $this->advancedSearchChecker->method('isEnabled')->willReturn(true); - $this->advancedSearchChecker - ->method('isEnabled') - ->willReturn(true); - - $resource - ->expects($this->once()) - ->method('getUri') + $resource->expects($this->once())->method('getUri') ->willReturn($documentUri); - $resource - ->method('getLabel') - ->willReturn('Resource Label'); - - $this->logger - ->expects($this->exactly(2)) - ->method('debug') - ->withConsecutive( - ['triggering index update on DataAccessControlChanged event'], - [ - $this->stringStartsWith( - 'Dispatching UpdateDataAccessControlInIndex for root resource' - ) - ] - ); + $resource->expects($this->once())->method('isClass') + ->willReturn($isClass); + + $this->logger->expects($this->once())->method('debug') + ->with('triggering index update on DataAccessControlChanged event'); - $this->ontology - ->expects($this->once()) - ->method('getResource') + $this->ontology->expects($this->once())->method('getResource') ->willReturn($resource); - $this->queueDispatcher - ->expects($this->once()) + $this->queueDispatcher->expects($this->once()) ->method('createTask') ->with( new UpdateDataAccessControlInIndex(), @@ -122,28 +105,20 @@ public function testHandleEventShouldCreateTaskSuccessfully(bool $isRecursive): false ); - $this->sut->handleEvent( - new DataAccessControlChangedEvent( - $documentUri, - [], - $isRecursive, - $isRecursive, - $documentUri - ) - ); + $this->sut->handleEvent(new DataAccessControlChangedEvent($documentUri, [], $isRecursive)); } - public function handleEventSuccessfulCasesDataProvider(): array + public function provideSuccessfulCases(): array { return [ 'case event is recursive and resource is NOT a class' => [ - true, + true, false ], 'case event is recursive and resource is a class' => [ - true, + true, true ], 'case event is NOT recursive and resource is not a class' => [ - false, + false, false ], ]; } @@ -155,6 +130,8 @@ public function testHandleEventShouldNotCreateTasks(): void $this->advancedSearchChecker->method('isEnabled')->willReturn(true); $resource->expects($this->never())->method('getUri'); + $resource->expects($this->once())->method('isClass') + ->willReturn(true); $this->logger->expects($this->once())->method('debug') ->with('triggering index update on DataAccessControlChanged event'); diff --git a/test/unit/models/classes/Csv/Service/ReaderTest.php b/test/unit/models/classes/Csv/Service/ReaderTest.php index 8de7993bf9..a16608e3d9 100644 --- a/test/unit/models/classes/Csv/Service/ReaderTest.php +++ b/test/unit/models/classes/Csv/Service/ReaderTest.php @@ -22,26 +22,10 @@ namespace oat\tao\test\unit\models\classes\Csv\Service; -use oat\generis\test\TestCase; -use oat\tao\model\Csv\Resource\Reader; -use League\Csv\Reader as LeagueCsvReader; -use PHPUnit\Framework\MockObject\MockObject; +use PHPUnit\Framework\TestCase; -// @TODO Test class in the next iteration class ReaderTest extends TestCase { - /** @var LeagueCsvReader|MockObject */ - private $leaguesCsv; - - /** @var Reader */ - private $sut; - - public function setUp(): void - { - $this->leaguesCsv = $this->createMock(LeagueCsvReader::class); - $this->sut = new Reader($this->leaguesCsv); - } - public function testCreateFromStream(): void { $this->markTestIncomplete('Test in the next iteration'); From 0628f0c26e9c07eea9aad6104d1aee903613c07e Mon Sep 17 00:00:00 2001 From: Andrei Shapiro Date: Thu, 2 Nov 2023 14:15:36 +0300 Subject: [PATCH 07/11] chore: add newline at the end of file --- models/classes/listener/DataAccessControlChangedListener.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/models/classes/listener/DataAccessControlChangedListener.php b/models/classes/listener/DataAccessControlChangedListener.php index 51f5519a69..807971d3b3 100644 --- a/models/classes/listener/DataAccessControlChangedListener.php +++ b/models/classes/listener/DataAccessControlChangedListener.php @@ -63,4 +63,4 @@ public function handleEvent(DataAccessControlChangedEvent $event): void $taskMessage ); } -} \ No newline at end of file +} From eaff4bd6ac7b21c63d4cb98a53920d5790b713d1 Mon Sep 17 00:00:00 2001 From: Andrei Shapiro Date: Thu, 2 Nov 2023 14:17:59 +0300 Subject: [PATCH 08/11] chore: use generis branch --- composer.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/composer.json b/composer.json index 17a40a987d..83fcb2941f 100644 --- a/composer.json +++ b/composer.json @@ -67,7 +67,7 @@ "oat-sa/jig": "~0.2", "oat-sa/composer-npm-bridge": "~0.4.2||dev-master", "oat-sa/oatbox-extension-installer": "~1.1||dev-master", - "oat-sa/generis": ">=15.29", + "oat-sa/generis": "dev-feature/adf-1590/optimize-permissions-task-queue as 99.99", "composer/package-versions-deprecated": "^1.11", "paragonie/random_compat": "^2.0", "phpdocumentor/reflection-docblock": "^5.2", From 8c59e1304270bc3b8dbd835dda908b1713e214a3 Mon Sep 17 00:00:00 2001 From: Andrei Shapiro Date: Thu, 2 Nov 2023 14:31:28 +0300 Subject: [PATCH 09/11] test: remove mocked method that does not exists --- .../Repository/RdfValueCollectionRepositoryTest.php | 8 -------- 1 file changed, 8 deletions(-) diff --git a/test/unit/models/classes/Lists/DataAccess/Repository/RdfValueCollectionRepositoryTest.php b/test/unit/models/classes/Lists/DataAccess/Repository/RdfValueCollectionRepositoryTest.php index ce086cab11..d23d41c8f9 100644 --- a/test/unit/models/classes/Lists/DataAccess/Repository/RdfValueCollectionRepositoryTest.php +++ b/test/unit/models/classes/Lists/DataAccess/Repository/RdfValueCollectionRepositoryTest.php @@ -131,10 +131,6 @@ public function testPersistUpdateNoChanges(): void ->expects(self::never()) ->method('setPropertyValue'); - $this->resourceMock - ->expects(self::never()) - ->method('updateUri'); - $value = new Value(666, 'uri', 'label'); $valueCollection = new ValueCollection('http://url', $value); @@ -162,10 +158,6 @@ public function testPersistUpdate(): void public function testPersistUpdateDifferentUris(): void { - $this->resourceMock - ->expects(self::once()) - ->method('updateUri'); - $value = new Value(666, 'uri1', 'label'); $value->setUri('uri2'); From fff70c69d4bff96de9eac52231abcc5822cd2381 Mon Sep 17 00:00:00 2001 From: Andrei Shapiro Date: Thu, 2 Nov 2023 14:44:12 +0300 Subject: [PATCH 10/11] chore: rollback --- .../Repository/RdfValueCollectionRepositoryTest.php | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/test/unit/models/classes/Lists/DataAccess/Repository/RdfValueCollectionRepositoryTest.php b/test/unit/models/classes/Lists/DataAccess/Repository/RdfValueCollectionRepositoryTest.php index d23d41c8f9..ce086cab11 100644 --- a/test/unit/models/classes/Lists/DataAccess/Repository/RdfValueCollectionRepositoryTest.php +++ b/test/unit/models/classes/Lists/DataAccess/Repository/RdfValueCollectionRepositoryTest.php @@ -131,6 +131,10 @@ public function testPersistUpdateNoChanges(): void ->expects(self::never()) ->method('setPropertyValue'); + $this->resourceMock + ->expects(self::never()) + ->method('updateUri'); + $value = new Value(666, 'uri', 'label'); $valueCollection = new ValueCollection('http://url', $value); @@ -158,6 +162,10 @@ public function testPersistUpdate(): void public function testPersistUpdateDifferentUris(): void { + $this->resourceMock + ->expects(self::once()) + ->method('updateUri'); + $value = new Value(666, 'uri1', 'label'); $value->setUri('uri2'); From 6a5cc2634bb05f2662ca0ccc4c4a60eba906273e Mon Sep 17 00:00:00 2001 From: Andrei Shapiro Date: Thu, 2 Nov 2023 16:29:05 +0300 Subject: [PATCH 11/11] chore: update generis version --- composer.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/composer.json b/composer.json index 83fcb2941f..fb1bf467a7 100644 --- a/composer.json +++ b/composer.json @@ -67,7 +67,7 @@ "oat-sa/jig": "~0.2", "oat-sa/composer-npm-bridge": "~0.4.2||dev-master", "oat-sa/oatbox-extension-installer": "~1.1||dev-master", - "oat-sa/generis": "dev-feature/adf-1590/optimize-permissions-task-queue as 99.99", + "oat-sa/generis": ">=15.32.0", "composer/package-versions-deprecated": "^1.11", "paragonie/random_compat": "^2.0", "phpdocumentor/reflection-docblock": "^5.2",