Skip to content

Move lib/ events to IEventDispatcher where possible - #39571

Merged
nickvergessen merged 8 commits into
masterfrom
bugfix/noid/move-lib-to-IEventDispatcher
Jul 27, 2023
Merged

Move lib/ events to IEventDispatcher where possible#39571
nickvergessen merged 8 commits into
masterfrom
bugfix/noid/move-lib-to-IEventDispatcher

Conversation

@nickvergessen

@nickvergessennickvergessen commented Jul 25, 2023

Copy link
Copy Markdown
Member

Checklist

@nickvergessennickvergessen added 3. to review Waiting for reviews technical debt 🧱 🤔🚀 labels Jul 25, 2023
@nickvergessennickvergessen added this to the Nextcloud 28 milestone Jul 25, 2023
@nickvergessennickvergessen self-assigned this Jul 25, 2023
@nickvergessennickvergessen added the pending documentation This pull request needs an associated documentation update label Jul 25, 2023
@nickvergessen
nickvergessenforce-pushed the bugfix/noid/move-lib-to-IEventDispatcher branch from 457f1fd to 8fba3b4CompareJuly 25, 2023 17:41
// notifications api to accept incoming user shares
$oldDispatcher->addListener('OCP\Share::postShare', function (OldGenericEvent $event) {
$dispatcher->addListener('OCP\Share::postShare', function ($event) {
if (!$event instanceof OldGenericEvent) {

Check notice

Code scanning / Psalm

RedundantConditionGivenDocblockType

OCP\EventDispatcher\Event does not contain Symfony\Component\EventDispatcher\GenericEvent
});
$oldDispatcher->addListener(IGroup::class . '::postAddUser', function (OldGenericEvent $event) {
$dispatcher->addListener(IGroup::class . '::postAddUser', function ($event) {
if (!$event instanceof OldGenericEvent) {

Check notice

Code scanning / Psalm

RedundantConditionGivenDocblockType

OCP\EventDispatcher\Event does not contain Symfony\Component\EventDispatcher\GenericEvent

$userBackendRegisteredEvent = new UserBackendRegistered($userBackend, $userPluginManager);
$legacyDispatcher->dispatch('OCA\\User_LDAP\\User\\User::postLDAPBackendAdded', $userBackendRegisteredEvent);
$dispatcher->dispatch('OCA\\User_LDAP\\User\\User::postLDAPBackendAdded', $userBackendRegisteredEvent);

Check notice

Code scanning / Psalm

DeprecatedMethod

The method OCP\EventDispatcher\IEventDispatcher::dispatch has been marked as deprecated
}

private function registerBackendDependents(IAppContainer $appContainer, EventDispatcherInterface $dispatcher) {
private function registerBackendDependents(IAppContainer $appContainer, IEventDispatcher $dispatcher) {

Check notice

Code scanning / Psalm

MissingReturnType

Method OCA\User_LDAP\AppInfo\Application::registerBackendDependents does not have a return type, expecting void
}

private function registerBackendDependents(IAppContainer $appContainer, EventDispatcherInterface $dispatcher) {
private function registerBackendDependents(IAppContainer $appContainer, IEventDispatcher $dispatcher) {

Check notice

Code scanning / Psalm

DeprecatedInterface

Interface OCP\AppFramework\IAppContainer is marked as deprecated
@nickvergessen
nickvergessenforce-pushed the bugfix/noid/move-lib-to-IEventDispatcher branch from 8fba3b4 to cd75dc0CompareJuly 26, 2023 06:38
@st3iny

Copy link
Copy Markdown
Member

Code is too deep for me. PHP CS check is unhappy though.

@st3iny
st3iny removed their request for review July 26, 2023 09:11
@nickvergessen
nickvergessenforce-pushed the bugfix/noid/move-lib-to-IEventDispatcher branch 3 times, most recently from 1193b43 to 978cef9CompareJuly 26, 2023 20:40

@come-nccome-nc left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Did you keep track of all removed legacy events to fill the upgrade documentation accordingly?

Comment threadlib/private/Share20/LegacyHooks.php Outdated
Signed-off-by: Joas Schilling <coding@schilljs.com>
Signed-off-by: Joas Schilling <coding@schilljs.com>
…itionalScripts
Signed-off-by: Joas Schilling <coding@schilljs.com>
… Manager
Signed-off-by: Joas Schilling <coding@schilljs.com>
Signed-off-by: Joas Schilling <coding@schilljs.com>
Signed-off-by: Joas Schilling <coding@schilljs.com>
Signed-off-by: Joas Schilling <coding@schilljs.com>
Signed-off-by: Joas Schilling <coding@schilljs.com>
@nickvergessen
nickvergessenforce-pushed the bugfix/noid/move-lib-to-IEventDispatcher branch from 978cef9 to 35c313eCompareJuly 27, 2023 07:58
@nickvergessen

nickvergessen commented Jul 27, 2023

Copy link
Copy Markdown
MemberAuthor

Did you keep track of all removed legacy events to fill the upgrade documentation accordingly?

Yes, will send a PR once the last remaining events are migrated. Todo list is:
https://github.com/nextcloud/server/issues?q=label%3A%22pending+documentation%22+author%3Anickvergessen+is%3Aclosed 😎

And can also be rechecked with the links on #38546 (comment) afterwards

@nickvergessen
nickvergessen merged commit 6b72217 into masterJul 27, 2023
@nickvergessen
nickvergessen deleted the bugfix/noid/move-lib-to-IEventDispatcher branch July 27, 2023 16:59
@nickvergessennickvergessen removed the pending documentation This pull request needs an associated documentation update label Jul 28, 2023
@nickvergessen

Copy link
Copy Markdown
MemberAuthor

Docs in nextcloud/documentation#10858

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

3. to reviewWaiting for reviewstechnical debt🧱 🤔🚀

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@nickvergessen@st3iny@ChristophWurst@github-advanced-security@come-nc