Skip to content

Migrate more apps to IEventDispatcher #39222

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
nickvergessen merged 3 commits into from
Jul 19, 2023
Merged

Migrate more apps to IEventDispatcher #39222

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a6bf935
fix(files_trashbin): Migrate to IEventDispatcher and emit as typed event
nickvergessen Jul 7, 2023
4877d0b
fix(loopup_server_connector): Migrate to IEventDispatcher
nickvergessen Jul 7, 2023
aa039c9
fix(workflowengine): Remove legacy event (deprecated since 17)
nickvergessen Jul 7, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions apps/files_trashbin/lib/Events/MoveToTrashEvent.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
* Event to allow other apps to disable the trash bin for specific files
*
* @package OCA\Files_Trashbin\Events
* @since 28.0.0 Dispatched as a typed event
*/
class MoveToTrashEvent extends Event {

Expand Down
11 changes: 6 additions & 5 deletions apps/files_trashbin/lib/Storage.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,13 +33,13 @@
use OCA\Files_Trashbin\Events\MoveToTrashEvent;
use OCA\Files_Trashbin\Trash\ITrashManager;
use OCP\Encryption\Exceptions\GenericEncryptionException;
use OCP\EventDispatcher\IEventDispatcher;
use OCP\Files\IRootFolder;
use OCP\Files\Mount\IMountPoint;
use OCP\Files\Node;
use OCP\Files\Storage\IStorage;
use OCP\ILogger;
use OCP\IUserManager;
use Symfony\Component\EventDispatcher\EventDispatcherInterface;

class Storage extends Wrapper {
/** @var IMountPoint */
Expand All @@ -51,7 +51,7 @@ class Storage extends Wrapper {
/** @var ILogger */
private $logger;

/** @var EventDispatcherInterface */
/** @var IEventDispatcher */
private $eventDispatcher;

/** @var IRootFolder */
Expand All @@ -69,15 +69,15 @@ class Storage extends Wrapper {
* @param ITrashManager $trashManager
* @param IUserManager|null $userManager
* @param ILogger|null $logger
* @param EventDispatcherInterface|null $eventDispatcher
* @param IEventDispatcher|null $eventDispatcher
* @param IRootFolder|null $rootFolder
*/
public function __construct(
$parameters,
ITrashManager $trashManager = null,
IUserManager $userManager = null,
ILogger $logger = null,
EventDispatcherInterface $eventDispatcher = null,
IEventDispatcher $eventDispatcher = null,
IRootFolder $rootFolder = null
) {
$this->mountPoint = $parameters['mountPoint'];
Expand Down Expand Up @@ -153,6 +153,7 @@ protected function shouldMoveToTrash($path) {

foreach ($nodes as $node) {
$event = $this->createMoveToTrashEvent($node);
$this->eventDispatcher->dispatchTyped($event);
$this->eventDispatcher->dispatch('OCA\Files_Trashbin::moveToTrash', $event);
if ($event->shouldMoveToTrashBin() === false) {
return false;
Expand Down Expand Up @@ -217,7 +218,7 @@ public static function setupStorage() {
\OC::$server->query(ITrashManager::class),
\OC::$server->getUserManager(),
\OC::$server->getLogger(),
\OC::$server->getEventDispatcher(),
\OC::$server->get(IEventDispatcher::class),
\OC::$server->getLazyRootFolder()
);
}, 1);
Expand Down
4 changes: 2 additions & 2 deletions apps/files_trashbin/tests/StorageTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
use OCA\Files_Trashbin\Trash\ITrashManager;
use OCP\AppFramework\Bootstrap\IBootContext;
use OCP\AppFramework\Utility\ITimeFactory;
use OCP\EventDispatcher\IEventDispatcher;
use OCP\Files\Cache\ICache;
use OCP\Files\Folder;
use OCP\Files\IRootFolder;
Expand All @@ -50,7 +51,6 @@
use OCP\IUserManager;
use OCP\Lock\ILockingProvider;
use OCP\Share\IShare;
use Symfony\Component\EventDispatcher\EventDispatcherInterface;
use Test\Traits\MountProviderTrait;

class TemporaryNoCross extends Temporary {
Expand Down Expand Up @@ -607,7 +607,7 @@ public function testShouldMoveToTrash($mountPoint, $path, $userExists, $appDisab
$userManager->expects($this->any())
->method('userExists')->willReturn($userExists);
$logger = $this->getMockBuilder(ILogger::class)->getMock();
$eventDispatcher = $this->createMock(EventDispatcherInterface::class);
$eventDispatcher = $this->createMock(IEventDispatcher::class);
$rootFolder = $this->createMock(IRootFolder::class);
$userFolder = $this->createMock(Folder::class);
$node = $this->getMockBuilder(Node::class)->disableOriginalConstructor()->getMock();
Expand Down
18 changes: 10 additions & 8 deletions apps/lookup_server_connector/lib/AppInfo/Application.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,9 @@
use OCP\AppFramework\Bootstrap\IBootContext;
use OCP\AppFramework\Bootstrap\IBootstrap;
use OCP\AppFramework\Bootstrap\IRegistrationContext;
use OCP\EventDispatcher\IEventDispatcher;
use OCP\IUser;
use Psr\Container\ContainerInterface;
use Symfony\Component\EventDispatcher\EventDispatcherInterface;
use Symfony\Component\EventDispatcher\GenericEvent;

class Application extends App implements IBootstrap {
Expand All @@ -56,15 +56,17 @@ public function boot(IBootContext $context): void {
/**
* @todo move the OCP events and then move the registration to `register`
*/
private function registerEventListeners(EventDispatcherInterface $dispatcher,
private function registerEventListeners(IEventDispatcher $dispatcher,
ContainerInterface $appContainer): void {
$dispatcher->addListener('OC\AccountManager::userUpdated', function (GenericEvent $event) use ($appContainer) {
/** @var IUser $user */
$user = $event->getSubject();
$dispatcher->addListener('OC\AccountManager::userUpdated', function ($event) use ($appContainer) {
Copy link
Contributor

@come-nc come-nc Jul 17, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is no typed event version for this one?
This change is not really updating to the new system, it’s still using the old one, no?

Copy link
Member Author

@nickvergessen nickvergessen Jul 18, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is no typed event version for this one?

No unluckily not.

This change is not really updating to the new system, it’s still using the old one, no?

For "listeners" there is no new/old system. It's the same. But yeah I'm not planing to migrate the things to typed events with the current work. I just want to free the way to be able to merge #38546 and that comes with a breaking API change, unless our wrapper is used when dispatching.
But for the ease of search and being sure, I'm move all code away from directly using the thirdparty.

if ($event instanceof GenericEvent) {

Check notice

Code scanning / Psalm

DocblockTypeContradiction

Cannot resolve types for $event - docblock-defined type OCP\EventDispatcher\Event does not contain Symfony\Component\EventDispatcher\GenericEvent
/** @var IUser $user */
$user = $event->getSubject();

/** @var UpdateLookupServer $updateLookupServer */
$updateLookupServer = $appContainer->get(UpdateLookupServer::class);
$updateLookupServer->userUpdated($user);
/** @var UpdateLookupServer $updateLookupServer */
$updateLookupServer = $appContainer->get(UpdateLookupServer::class);
$updateLookupServer->userUpdated($user);
}
});
}
}
10 changes: 0 additions & 10 deletions apps/workflowengine/lib/Manager.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,8 +66,6 @@
use OCP\WorkflowEngine\IManager;
use OCP\WorkflowEngine\IOperation;
use OCP\WorkflowEngine\IRuleMatcher;
use Symfony\Component\EventDispatcher\EventDispatcherInterface as LegacyDispatcher;
use Symfony\Component\EventDispatcher\GenericEvent;

class Manager implements IManager {
/** @var IStorage */
Expand All @@ -94,9 +92,6 @@ class Manager implements IManager {
/** @var IL10N */
protected $l;

/** @var LegacyDispatcher */
protected $legacyEventDispatcher;

/** @var IEntity[] */
protected $registeredEntities = [];

Expand Down Expand Up @@ -126,7 +121,6 @@ public function __construct(
IDBConnection $connection,
IServerContainer $container,
IL10N $l,
LegacyDispatcher $eventDispatcher,
ILogger $logger,
IUserSession $session,
IEventDispatcher $dispatcher,
Expand All @@ -136,7 +130,6 @@ public function __construct(
$this->connection = $connection;
$this->container = $container;
$this->l = $l;
$this->legacyEventDispatcher = $eventDispatcher;
$this->logger = $logger;
$this->operationsByScope = new CappedMemoryCache(64);
$this->session = $session;
Expand Down Expand Up @@ -694,7 +687,6 @@ public function formatOperation(array $operation): array {
*/
public function getEntitiesList(): array {
$this->dispatcher->dispatchTyped(new RegisterEntitiesEvent($this));
$this->legacyEventDispatcher->dispatch(IManager::EVENT_NAME_REG_ENTITY, new GenericEvent($this));
Copy link
Contributor

@come-nc come-nc Jul 17, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code in applications which are registering a listener for this, will they get an error or just silently won’t be called anymore?

Copy link
Member Author

@nickvergessen nickvergessen Jul 18, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"Silently" won't be called anymore after 3+ years of deprecation. It is documented in the changelogs never the less:
https://docs.nextcloud.com/server/latest/developer_manual/app_publishing_maintenance/app_upgrade_guide/upgrade_to_28.html#removed-events


return array_values(array_merge($this->getBuildInEntities(), $this->registeredEntities));
}
Expand All @@ -704,7 +696,6 @@ public function getEntitiesList(): array {
*/
public function getOperatorList(): array {
$this->dispatcher->dispatchTyped(new RegisterOperationsEvent($this));
$this->legacyEventDispatcher->dispatch(IManager::EVENT_NAME_REG_OPERATION, new GenericEvent($this));

return array_merge($this->getBuildInOperators(), $this->registeredOperators);
}
Expand All @@ -714,7 +705,6 @@ public function getOperatorList(): array {
*/
public function getCheckList(): array {
$this->dispatcher->dispatchTyped(new RegisterChecksEvent($this));
$this->legacyEventDispatcher->dispatch(IManager::EVENT_NAME_REG_CHECK, new GenericEvent($this));

return array_merge($this->getBuildInChecks(), $this->registeredChecks);
}
Expand Down
12 changes: 4 additions & 8 deletions apps/workflowengine/tests/ManagerTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@
use OCP\IUserManager;
use OCP\IUserSession;
use OCP\SystemTag\ISystemTagManager;
use OCP\WorkflowEngine\Events\RegisterEntitiesEvent;
use OCP\WorkflowEngine\ICheck;
use OCP\WorkflowEngine\IEntity;
use OCP\WorkflowEngine\IEntityEvent;
Expand All @@ -67,8 +68,6 @@ class ManagerTest extends TestCase {
protected $db;
/** @var \PHPUnit\Framework\MockObject\MockObject|ILogger */
protected $logger;
/** @var \PHPUnit\Framework\MockObject\MockObject|EventDispatcherInterface */
protected $legacyDispatcher;
/** @var MockObject|IServerContainer */
protected $container;
/** @var MockObject|IUserSession */
Expand All @@ -94,7 +93,6 @@ protected function setUp(): void {
return vsprintf($text, $parameters);
});

$this->legacyDispatcher = $this->createMock(EventDispatcherInterface::class);
$this->logger = $this->createMock(ILogger::class);
$this->session = $this->createMock(IUserSession::class);
$this->dispatcher = $this->createMock(IEventDispatcher::class);
Expand All @@ -105,7 +103,6 @@ protected function setUp(): void {
\OC::$server->getDatabaseConnection(),
$this->container,
$this->l,
$this->legacyDispatcher,
$this->logger,
$this->session,
$this->dispatcher,
Expand Down Expand Up @@ -532,10 +529,9 @@ public function testGetEntitiesList() {
/** @var MockObject|IEntity $extraEntity */
$extraEntity = $this->createMock(IEntity::class);

$this->legacyDispatcher->expects($this->once())
->method('dispatch')
->with('OCP\WorkflowEngine::registerEntities', $this->anything())
->willReturnCallback(function () use ($extraEntity) {
$this->dispatcher->expects($this->once())
->method('dispatchTyped')
->willReturnCallback(function (RegisterEntitiesEvent $e) use ($extraEntity) {
$this->manager->registerEntity($extraEntity);
});

Expand Down
15 changes: 0 additions & 15 deletions lib/public/WorkflowEngine/IManager.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,21 +44,6 @@ interface IManager {
*/
public const MAX_OPERATION_VALUE_BYTES = 4096;

/**
* @deprecated 17.0.0 Will be removed in NC19. Use the dedicated events in OCP\WorkflowEngine\Events
*/
public const EVENT_NAME_REG_OPERATION = 'OCP\WorkflowEngine::registerOperations';

/**
* @deprecated 17.0.0
*/
public const EVENT_NAME_REG_ENTITY = 'OCP\WorkflowEngine::registerEntities';

/**
* @deprecated 17.0.0
*/
public const EVENT_NAME_REG_CHECK = 'OCP\WorkflowEngine::registerChecks';

/**
* Listen to `OCP\WorkflowEngine\Events\RegisterEntitiesEvent` at the
* IEventDispatcher for registering your entities.
Expand Down